diff options
Diffstat (limited to 'spec/models')
75 files changed, 2677 insertions, 574 deletions
diff --git a/spec/models/analytics/cycle_analytics/aggregation_spec.rb b/spec/models/analytics/cycle_analytics/aggregation_spec.rb new file mode 100644 index 00000000000..4bf737df56a --- /dev/null +++ b/spec/models/analytics/cycle_analytics/aggregation_spec.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:group).required } + end + + describe 'validations' do + it { is_expected.not_to validate_presence_of(:group) } + it { is_expected.not_to validate_presence_of(:enabled) } + + %i[incremental_runtimes_in_seconds incremental_processed_records last_full_run_runtimes_in_seconds last_full_run_processed_records].each do |column| + it "validates the array length of #{column}" do + record = described_class.new(column => [1] * 11) + + expect(record).to be_invalid + expect(record.errors).to have_key(column) + end + end + end + + describe '#safe_create_for_group' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + + it 'creates the aggregation record' do + record = described_class.safe_create_for_group(group) + + expect(record).to be_persisted + end + + context 'when non top-level group is given' do + it 'creates the aggregation record for the top-level group' do + record = described_class.safe_create_for_group(subgroup) + + expect(record).to be_persisted + end + end + + context 'when the record is already present' do + it 'does nothing' do + described_class.safe_create_for_group(group) + + expect do + described_class.safe_create_for_group(group) + described_class.safe_create_for_group(subgroup) + end.not_to change { described_class.count } + end + end + end + + describe '#load_batch' do + let!(:aggregation1) { create(:cycle_analytics_aggregation, last_incremental_run_at: nil, last_consistency_check_updated_at: 3.days.ago).reload } + let!(:aggregation2) { create(:cycle_analytics_aggregation, last_incremental_run_at: 5.days.ago).reload } + let!(:aggregation3) { create(:cycle_analytics_aggregation, last_incremental_run_at: nil, last_consistency_check_updated_at: 2.days.ago).reload } + let!(:aggregation4) { create(:cycle_analytics_aggregation, last_incremental_run_at: 10.days.ago).reload } + + before do + create(:cycle_analytics_aggregation, :disabled) # disabled rows are skipped + create(:cycle_analytics_aggregation, last_incremental_run_at: 1.day.ago, last_consistency_check_updated_at: 1.hour.ago) # "early" rows are filtered out + end + + it 'loads records in priority order' do + batch = described_class.load_batch(2.days.ago).to_a + + expect(batch.size).to eq(4) + first_two = batch.first(2) + last_two = batch.last(2) + + # Using match_array because the order can be undeterministic for nil values. + expect(first_two).to match_array([aggregation1, aggregation3]) + expect(last_two).to eq([aggregation4, aggregation2]) + end + + context 'when loading batch for last_consistency_check_updated_at' do + it 'loads records in priority order' do + batch = described_class.load_batch(1.day.ago, :last_consistency_check_updated_at).to_a + + expect(batch.size).to eq(4) + first_two = batch.first(2) + last_two = batch.last(2) + + expect(first_two).to match_array([aggregation2, aggregation4]) + expect(last_two).to eq([aggregation1, aggregation3]) + end + end + end + + describe '#estimated_next_run_at' do + around do |example| + travel_to(Time.utc(2019, 3, 17, 13, 3)) { example.run } + end + + # aggregation runs on every 10 minutes + let(:minutes_until_next_aggregation) { 7.minutes } + + context 'when aggregation was not yet executed for the given group' do + let(:aggregation) { create(:cycle_analytics_aggregation, last_incremental_run_at: nil) } + + it { expect(aggregation.estimated_next_run_at).to eq(nil) } + end + + context 'when aggregation was already run' do + let!(:other_aggregation1) { create(:cycle_analytics_aggregation, last_incremental_run_at: 10.minutes.ago) } + let!(:other_aggregation2) { create(:cycle_analytics_aggregation, last_incremental_run_at: 15.minutes.ago) } + let!(:aggregation) { create(:cycle_analytics_aggregation, last_incremental_run_at: 5.minutes.ago) } + + it 'returns the duration between the previous run timestamp and the earliest last_incremental_run_at' do + expect(aggregation.estimated_next_run_at).to eq((10.minutes + minutes_until_next_aggregation).from_now) + end + + context 'when the aggregation has persisted previous runtimes' do + before do + aggregation.update!(incremental_runtimes_in_seconds: [30, 60, 90]) + end + + it 'adds the runtime to the estimation' do + expect(aggregation.estimated_next_run_at).to eq((10.minutes.seconds + 60.seconds + minutes_until_next_aggregation).from_now) + end + end + end + + context 'when no records are present in the DB' do + it 'returns nil' do + expect(described_class.new.estimated_next_run_at).to eq(nil) + end + end + + context 'when only one aggregation record present' do + let!(:aggregation) { create(:cycle_analytics_aggregation, last_incremental_run_at: 5.minutes.ago) } + + it 'returns the minutes until the next aggregation' do + expect(aggregation.estimated_next_run_at).to eq(minutes_until_next_aggregation.from_now) + end + end + end +end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 9c9a048999c..c1cd44e9007 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -104,6 +104,18 @@ RSpec.describe ApplicationRecord do end end + describe '.where_not_exists' do + it 'produces a WHERE NOT EXISTS query' do + create(:user, :two_factor_via_u2f) + user_2 = create(:user) + + expect( + User.where_not_exists( + U2fRegistration.where(U2fRegistration.arel_table[:user_id].eq(User.arel_table[:id]))) + ).to match_array([user_2]) + end + end + describe '.transaction', :delete do it 'opens a new transaction' do expect(described_class.connection.transaction_open?).to be false diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index a962703d460..70331e8d78a 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -76,6 +76,8 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_numericality_of(:container_registry_delete_tags_service_timeout).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to allow_value(true).for(:container_registry_expiration_policies_caching) } + it { is_expected.to allow_value(false).for(:container_registry_expiration_policies_caching) } it { is_expected.to validate_numericality_of(:container_registry_import_max_tags_count).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_import_max_retries).only_integer.is_greater_than_or_equal_to(0) } @@ -141,7 +143,7 @@ RSpec.describe ApplicationSetting do it { is_expected.not_to allow_value('default' => 101).for(:repository_storages_weighted).with_message("value for 'default' must be between 0 and 100") } it { is_expected.not_to allow_value('default' => 100, shouldntexist: 50).for(:repository_storages_weighted).with_message("can't include: shouldntexist") } - %i[notes_create_limit user_email_lookup_limit users_get_by_id_limit].each do |setting| + %i[notes_create_limit search_rate_limit search_rate_limit_unauthenticated users_get_by_id_limit].each do |setting| it { is_expected.to allow_value(400).for(setting) } it { is_expected.not_to allow_value('two').for(setting) } it { is_expected.not_to allow_value(nil).for(setting) } diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index d981189c6f1..b0bfdabe13c 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -23,6 +23,8 @@ RSpec.describe BroadcastMessage do it { is_expected.to allow_value(1).for(:broadcast_type) } it { is_expected.not_to allow_value(nil).for(:broadcast_type) } + it { is_expected.not_to allow_value(nil).for(:target_access_levels) } + it { is_expected.to validate_inclusion_of(:target_access_levels).in_array(described_class::ALLOWED_TARGET_ACCESS_LEVELS) } end shared_examples 'time constrainted' do |broadcast_type| @@ -60,7 +62,7 @@ RSpec.describe BroadcastMessage do subject.call - Timecop.travel(3.weeks) do + travel_to(3.weeks.from_now) do subject.call end end @@ -71,7 +73,7 @@ RSpec.describe BroadcastMessage do expect(subject.call).to match_array([message]) expect(described_class.cache).to receive(:expire).and_call_original - Timecop.travel(1.week) do + travel_to(1.week.from_now) do 2.times { expect(subject.call).to be_empty } end end @@ -94,7 +96,7 @@ RSpec.describe BroadcastMessage do expect(subject.call.length).to eq(1) - Timecop.travel(future.starts_at) do + travel_to(future.starts_at + 1.second) do expect(subject.call.length).to eq(2) end end @@ -175,12 +177,112 @@ RSpec.describe BroadcastMessage do end end + shared_examples "matches with user access level" do |broadcast_type| + let_it_be(:target_access_levels) { [Gitlab::Access::GUEST] } + + let(:feature_flag_state) { true } + + before do + stub_feature_flags(role_targeted_broadcast_messages: feature_flag_state) + end + + context 'when feature flag is disabled' do + let(:feature_flag_state) { false } + + context 'when message is role-targeted' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: target_access_levels, broadcast_type: broadcast_type) } + + it 'does not return the message' do + expect(subject.call(nil, Gitlab::Access::GUEST)).to be_empty + end + end + + context 'when message is not role-targeted' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: [], broadcast_type: broadcast_type) } + + it 'returns the message' do + expect(subject.call(nil, Gitlab::Access::GUEST)).to include(message) + end + end + end + + context 'when target_access_levels is empty' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: [], broadcast_type: broadcast_type) } + + it 'returns the message if user access level is not nil' do + expect(subject.call(nil, Gitlab::Access::MINIMAL_ACCESS)).to include(message) + end + + it 'returns the message if user access level is nil' do + expect(subject.call).to include(message) + end + end + + context 'when target_access_levels is not empty' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: target_access_levels, broadcast_type: broadcast_type) } + + it "does not return the message if user access level is nil" do + expect(subject.call).to be_empty + end + + it "returns the message if user access level is in target_access_levels" do + expect(subject.call(nil, Gitlab::Access::GUEST)).to include(message) + end + + it "does not return the message if user access level is not in target_access_levels" do + expect(subject.call(nil, Gitlab::Access::MINIMAL_ACCESS)).to be_empty + end + end + end + + shared_examples "handles stale cache data gracefully" do + # Regression test for https://gitlab.com/gitlab-org/gitlab/-/issues/353076 + context 'when cache returns stale data (e.g. nil target_access_levels)' do + let(:message) { build(:broadcast_message, :banner, target_access_levels: nil) } + let(:cache) { Gitlab::JsonCache.new } + + before do + cache.write(described_class::BANNER_CACHE_KEY, [message]) + allow(BroadcastMessage).to receive(:cache) { cache } + end + + it 'does not raise error (e.g. NoMethodError from nil.empty?)' do + expect { subject.call }.not_to raise_error + end + + context 'when feature flag is disabled' do + it 'does not raise error (e.g. NoMethodError from nil.empty?)' do + stub_feature_flags(role_targeted_broadcast_messages: false) + + expect { subject.call }.not_to raise_error + end + end + end + end + describe '.current', :use_clean_rails_memory_store_caching do - subject { -> (path = nil) { described_class.current(path) } } + subject do + -> (path = nil, user_access_level = nil) do + described_class.current(current_path: path, user_access_level: user_access_level) + end + end it_behaves_like 'time constrainted', :banner it_behaves_like 'message cache', :banner it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + it_behaves_like 'handles stale cache data gracefully' + + context 'when message is from cache' do + before do + subject.call + end + + it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + it_behaves_like 'matches with current path', :notification + it_behaves_like 'matches with user access level', :notification + end it 'returns both types' do banner_message = create(:broadcast_message, broadcast_type: :banner) @@ -191,11 +293,26 @@ RSpec.describe BroadcastMessage do end describe '.current_banner_messages', :use_clean_rails_memory_store_caching do - subject { -> (path = nil) { described_class.current_banner_messages(path) } } + subject do + -> (path = nil, user_access_level = nil) do + described_class.current_banner_messages(current_path: path, user_access_level: user_access_level) + end + end it_behaves_like 'time constrainted', :banner it_behaves_like 'message cache', :banner it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + it_behaves_like 'handles stale cache data gracefully' + + context 'when message is from cache' do + before do + subject.call + end + + it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + end it 'only returns banners' do banner_message = create(:broadcast_message, broadcast_type: :banner) @@ -206,11 +323,26 @@ RSpec.describe BroadcastMessage do end describe '.current_notification_messages', :use_clean_rails_memory_store_caching do - subject { -> (path = nil) { described_class.current_notification_messages(path) } } + subject do + -> (path = nil, user_access_level = nil) do + described_class.current_notification_messages(current_path: path, user_access_level: user_access_level) + end + end it_behaves_like 'time constrainted', :notification it_behaves_like 'message cache', :notification it_behaves_like 'matches with current path', :notification + it_behaves_like 'matches with user access level', :notification + it_behaves_like 'handles stale cache data gracefully' + + context 'when message is from cache' do + before do + subject.call + end + + it_behaves_like 'matches with current path', :notification + it_behaves_like 'matches with user access level', :notification + end it 'only returns notifications' do notification_message = create(:broadcast_message, broadcast_type: :notification) @@ -286,9 +418,9 @@ RSpec.describe BroadcastMessage do it 'flushes the Redis cache' do message = create(:broadcast_message) - expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY) - expect(Rails.cache).to receive(:delete).with(described_class::BANNER_CACHE_KEY) - expect(Rails.cache).to receive(:delete).with(described_class::NOTIFICATION_CACHE_KEY) + expect(Rails.cache).to receive(:delete).with("#{described_class::CACHE_KEY}:#{Gitlab.revision}") + expect(Rails.cache).to receive(:delete).with("#{described_class::BANNER_CACHE_KEY}:#{Gitlab.revision}") + expect(Rails.cache).to receive(:delete).with("#{described_class::NOTIFICATION_CACHE_KEY}:#{Gitlab.revision}") message.flush_redis_cache end diff --git a/spec/models/bulk_imports/export_status_spec.rb b/spec/models/bulk_imports/export_status_spec.rb index 48f32a2092a..f945ad12244 100644 --- a/spec/models/bulk_imports/export_status_spec.rb +++ b/spec/models/bulk_imports/export_status_spec.rb @@ -55,6 +55,17 @@ RSpec.describe BulkImports::ExportStatus do expect(subject.failed?).to eq(false) end end + + context 'when export status is not present' do + let(:response_double) do + double(parsed_response: []) + end + + it 'returns true' do + expect(subject.failed?).to eq(true) + expect(subject.error).to eq('Empty export status response') + end + end end describe '#error' do diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 6fde55103f8..7c3c02a5ab7 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Ci::Bridge do let_it_be(:target_project) { create(:project, name: 'project', namespace: create(:namespace, name: 'my')) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + before_all do + create(:ci_pipeline_variable, pipeline: pipeline, key: 'PVAR1', value: 'PVAL1') + end + let(:bridge) do create(:ci_bridge, :variables, status: :created, options: options, @@ -215,6 +219,70 @@ RSpec.describe Ci::Bridge do .to include(key: 'EXPANDED', value: '$EXPANDED') end end + + context 'forward variables' do + using RSpec::Parameterized::TableSyntax + + where(:yaml_variables, :pipeline_variables, :ff, :variables) do + nil | nil | true | %w[BRIDGE] + nil | false | true | %w[BRIDGE] + nil | true | true | %w[BRIDGE PVAR1] + false | nil | true | %w[] + false | false | true | %w[] + false | true | true | %w[PVAR1] + true | nil | true | %w[BRIDGE] + true | false | true | %w[BRIDGE] + true | true | true | %w[BRIDGE PVAR1] + nil | nil | false | %w[BRIDGE] + nil | false | false | %w[BRIDGE] + nil | true | false | %w[BRIDGE] + false | nil | false | %w[BRIDGE] + false | false | false | %w[BRIDGE] + false | true | false | %w[BRIDGE] + true | nil | false | %w[BRIDGE] + true | false | false | %w[BRIDGE] + true | true | false | %w[BRIDGE] + end + + with_them do + let(:options) do + { + trigger: { + project: 'my/project', + branch: 'master', + forward: { yaml_variables: yaml_variables, + pipeline_variables: pipeline_variables }.compact + } + } + end + + before do + stub_feature_flags(ci_trigger_forward_variables: ff) + end + + it 'returns variables according to the forward value' do + expect(bridge.downstream_variables.map { |v| v[:key] }).to contain_exactly(*variables) + end + end + + context 'when sending a variable via both yaml and pipeline' do + let(:pipeline) { create(:ci_pipeline, project: project) } + + let(:options) do + { trigger: { project: 'my/project', forward: { pipeline_variables: true } } } + end + + before do + create(:ci_pipeline_variable, pipeline: pipeline, key: 'BRIDGE', value: 'new value') + end + + it 'uses the pipeline variable' do + expect(bridge.downstream_variables).to contain_exactly( + { key: 'BRIDGE', value: 'new value' } + ) + end + end + end end describe 'metadata support' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 90298f0e973..240b932638a 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3510,6 +3510,38 @@ RSpec.describe Ci::Build do end end + context 'for harbor integration' do + let(:harbor_integration) { create(:harbor_integration) } + + let(:harbor_variables) do + [ + { key: 'HARBOR_URL', value: harbor_integration.url, public: true, masked: false }, + { key: 'HARBOR_PROJECT', value: harbor_integration.project_name, public: true, masked: false }, + { key: 'HARBOR_USERNAME', value: harbor_integration.username, public: true, masked: false }, + { key: 'HARBOR_PASSWORD', value: harbor_integration.password, public: false, masked: true } + ] + end + + context 'when harbor_integration exists' do + before do + build.project.update!(harbor_integration: harbor_integration) + end + + it 'includes harbor variables' do + is_expected.to include(*harbor_variables) + end + end + + context 'when harbor_integration does not exist' do + it 'does not include harbor variables' do + expect(subject.find { |v| v[:key] == 'HARBOR_URL'}).to be_nil + expect(subject.find { |v| v[:key] == 'HARBOR_PROJECT_NAME'}).to be_nil + expect(subject.find { |v| v[:key] == 'HARBOR_USERNAME'}).to be_nil + expect(subject.find { |v| v[:key] == 'HARBOR_PASSWORD'}).to be_nil + end + end + end + context 'when build has dependency which has dotenv variable' do let!(:prepare) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 1, options: { dependencies: [prepare.name] }) } diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb index 4cb3b9eef0c..3a4b836e453 100644 --- a/spec/models/ci/group_variable_spec.rb +++ b/spec/models/ci/group_variable_spec.rb @@ -43,6 +43,14 @@ RSpec.describe Ci::GroupVariable do end end + describe '.for_groups' do + let_it_be(:group) { create(:group) } + let_it_be(:group_variable) { create(:ci_group_variable, group: group) } + let_it_be(:other_variable) { create(:ci_group_variable) } + + it { expect(described_class.for_groups([group.id])).to eq([group_variable]) } + end + it_behaves_like 'cleanup by a loose foreign key' do let!(:model) { create(:ci_group_variable) } diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 0f4f148775e..3c295fb345b 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -228,6 +228,66 @@ RSpec.describe Ci::PipelineSchedule do end end + describe '#for_tag?' do + context 'when the target is a tag' do + before do + subject.ref = 'refs/tags/v1.0' + end + + it { expect(subject.for_tag?).to eq(true) } + end + + context 'when the target is a branch' do + before do + subject.ref = 'refs/heads/main' + end + + it { expect(subject.for_tag?).to eq(false) } + end + + context 'when there is no ref' do + before do + subject.ref = nil + end + + it { expect(subject.for_tag?).to eq(false) } + end + end + + describe '#ref_for_display' do + context 'when the target is a tag' do + before do + subject.ref = 'refs/tags/v1.0' + end + + it { expect(subject.ref_for_display).to eq('v1.0') } + end + + context 'when the target is a branch' do + before do + subject.ref = 'refs/heads/main' + end + + it { expect(subject.ref_for_display).to eq('main') } + end + + context 'when the ref is ambiguous' do + before do + subject.ref = 'release-2.8' + end + + it { expect(subject.ref_for_display).to eq('release-2.8') } + end + + context 'when there is no ref' do + before do + subject.ref = nil + end + + it { expect(subject.ref_for_display).to eq(nil) } + end + end + context 'loose foreign key on ci_pipeline_schedules.project_id' do it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index c29cc04e0e9..294ec07ee3e 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -438,15 +438,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { expect(pipeline).not_to be_merge_request } end - - context 'when the feature flag is disabled' do - before do - stub_feature_flags(ci_pipeline_merge_request_presence_check: false) - pipeline.update!(merge_request_id: non_existing_record_id) - end - - it { expect(pipeline).to be_merge_request } - end end describe '#detached_merge_request_pipeline?' do @@ -2890,6 +2881,34 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '.cancelable' do + subject { described_class.cancelable } + + shared_examples 'containing the pipeline' do |status| + context "when it's #{status} pipeline" do + let!(:pipeline) { create(:ci_pipeline, status: status) } + + it { is_expected.to contain_exactly(pipeline) } + end + end + + shared_examples 'not containing the pipeline' do |status| + context "when it's #{status} pipeline" do + let!(:pipeline) { create(:ci_pipeline, status: status) } + + it { is_expected.to be_empty } + end + end + + %i[running pending waiting_for_resource preparing created scheduled manual].each do |status| + it_behaves_like 'containing the pipeline', status + end + + %i[failed success skipped canceled].each do |status| + it_behaves_like 'not containing the pipeline', status + end + end + describe '#retry_failed' do subject(:latest_status) { pipeline.latest_statuses.pluck(:status) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index eb29db697a5..0620bb1ffc5 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -951,7 +951,7 @@ RSpec.describe Ci::Runner do let!(:last_update) { runner.ensure_runner_queue_value } before do - Ci::UpdateRunnerService.new(runner).update(description: 'new runner') # rubocop: disable Rails/SaveBang + Ci::Runners::UpdateRunnerService.new(runner).update(description: 'new runner') # rubocop: disable Rails/SaveBang end it 'sets a new last_update value' do diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb index ae57b63e7a4..4382385aaf5 100644 --- a/spec/models/ci/secure_file_spec.rb +++ b/spec/models/ci/secure_file_spec.rb @@ -17,6 +17,10 @@ RSpec.describe Ci::SecureFile do it_behaves_like 'having unique enum values' + it_behaves_like 'includes Limitable concern' do + subject { build(:ci_secure_file, project: create(:project)) } + end + describe 'validations' do it { is_expected.to validate_presence_of(:checksum) } it { is_expected.to validate_presence_of(:file_store) } diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb index 993afd47a57..358000ee174 100644 --- a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb +++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb @@ -6,10 +6,10 @@ RSpec.describe BatchDestroyDependentAssociations do class TestProject < ActiveRecord::Base self.table_name = 'projects' - has_many :builds, dependent: :destroy + has_many :builds has_many :notification_settings, as: :source, dependent: :delete_all has_many :pages_domains - has_many :todos + has_many :todos, dependent: :destroy include BatchDestroyDependentAssociations end @@ -18,7 +18,7 @@ RSpec.describe BatchDestroyDependentAssociations do let_it_be(:project) { TestProject.new } it 'returns the right associations' do - expect(project.dependent_associations_to_destroy.map(&:name)).to match_array([:builds]) + expect(project.dependent_associations_to_destroy.map(&:name)).to match_array([:todos]) end end @@ -26,36 +26,35 @@ RSpec.describe BatchDestroyDependentAssociations do let_it_be(:project) { create(:project) } let_it_be(:build) { create(:ci_build, project: project) } let_it_be(:notification_setting) { create(:notification_setting, project: project) } + let_it_be(:note) { create(:note, project: project) } - let!(:todos) { create(:todo, project: project) } + it 'destroys multiple notes' do + create(:note, project: project) - it 'destroys multiple builds' do - create(:ci_build, project: project) - - expect(Ci::Build.count).to eq(2) + expect(Note.count).to eq(2) project.destroy_dependent_associations_in_batches - expect(Ci::Build.count).to eq(0) + expect(Note.count).to eq(0) end - it 'destroys builds in batches' do - expect(project).to receive_message_chain(:builds, :find_each).and_yield(build) - expect(build).to receive(:destroy).and_call_original + it 'destroys note in batches' do + expect(project).to receive_message_chain(:notes, :find_each).and_yield(note) + expect(note).to receive(:destroy).and_call_original project.destroy_dependent_associations_in_batches - expect(Ci::Build.count).to eq(0) - expect(Todo.count).to eq(1) + expect(Ci::Build.count).to eq(1) + expect(Note.count).to eq(0) expect(User.count).to be > 0 expect(NotificationSetting.count).to eq(User.count) end it 'excludes associations' do - project.destroy_dependent_associations_in_batches(exclude: [:builds]) + project.destroy_dependent_associations_in_batches(exclude: [:notes]) + expect(Note.count).to eq(1) expect(Ci::Build.count).to eq(1) - expect(Todo.count).to eq(1) expect(User.count).to be > 0 expect(NotificationSetting.count).to eq(User.count) end diff --git a/spec/models/concerns/blocks_json_serialization_spec.rb b/spec/models/concerns/blocks_json_serialization_spec.rb deleted file mode 100644 index d811b47fa35..00000000000 --- a/spec/models/concerns/blocks_json_serialization_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe BlocksJsonSerialization do - before do - stub_const('DummyModel', Class.new) - DummyModel.class_eval do - include BlocksJsonSerialization - end - end - - it 'blocks as_json' do - expect { DummyModel.new.as_json } - .to raise_error(described_class::JsonSerializationError, /DummyModel/) - end - - it 'blocks to_json' do - expect { DummyModel.new.to_json } - .to raise_error(described_class::JsonSerializationError, /DummyModel/) - end -end diff --git a/spec/models/concerns/blocks_unsafe_serialization_spec.rb b/spec/models/concerns/blocks_unsafe_serialization_spec.rb new file mode 100644 index 00000000000..5c8f5035a58 --- /dev/null +++ b/spec/models/concerns/blocks_unsafe_serialization_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BlocksUnsafeSerialization do + before do + stub_const('DummyModel', Class.new) + DummyModel.class_eval do + include ActiveModel::Serializers::JSON + include BlocksUnsafeSerialization + end + end + + it_behaves_like 'blocks unsafe serialization' do + let(:object) { DummyModel.new } + end +end diff --git a/spec/models/concerns/ci/has_deployment_name_spec.rb b/spec/models/concerns/ci/has_deployment_name_spec.rb new file mode 100644 index 00000000000..8c7338638b1 --- /dev/null +++ b/spec/models/concerns/ci/has_deployment_name_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::HasDeploymentName do + describe 'deployment_name?' do + let(:build) { create(:ci_build) } + + subject { build.branch? } + + it 'does detect deployment names' do + build.name = 'deployment' + + expect(build.deployment_name?).to be_truthy + end + + it 'does detect partial deployment names' do + build.name = 'do a really cool deploy' + + expect(build.deployment_name?).to be_truthy + end + + it 'does not detect non-deployment names' do + build.name = 'testing' + + expect(build.deployment_name?).to be_falsy + end + + it 'is case insensitive' do + build.name = 'DEPLOY' + expect(build.deployment_name?).to be_truthy + end + end +end diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 7fa55184cf1..bd1afe844ac 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -12,16 +12,28 @@ RSpec.describe DeploymentPlatform do let(:group) { create(:group) } let(:project) { create(:project, group: group) } + shared_examples 'certificate_based_clusters is disabled' do + before do + stub_feature_flags(certificate_based_clusters: false) + end + + it { is_expected.to be_nil } + end + shared_examples 'matching environment scope' do it 'returns environment specific cluster' do is_expected.to eq(cluster.platform_kubernetes) end + + it_behaves_like 'certificate_based_clusters is disabled' end shared_examples 'not matching environment scope' do it 'returns default cluster' do is_expected.to eq(default_cluster.platform_kubernetes) end + + it_behaves_like 'certificate_based_clusters is disabled' end context 'multiple clusters use the same management project' do diff --git a/spec/models/concerns/issuable_link_spec.rb b/spec/models/concerns/issuable_link_spec.rb new file mode 100644 index 00000000000..7be6d8a074d --- /dev/null +++ b/spec/models/concerns/issuable_link_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe IssuableLink do + let(:test_class) do + Class.new(ApplicationRecord) do + include IssuableLink + + self.table_name = 'issue_links' + + belongs_to :source, class_name: 'Issue' + belongs_to :target, class_name: 'Issue' + + def self.name + 'TestClass' + end + end + end + + describe '.inverse_link_type' do + it 'returns the inverse type of link' do + expect(test_class.inverse_link_type('relates_to')).to eq('relates_to') + end + end + + describe '.issuable_type' do + let_it_be(:source_issue) { create(:issue) } + let_it_be(:target_issue) { create(:issue) } + + before do + test_class.create!(source: source_issue, target: target_issue) + end + + context 'when opposite relation already exists' do + it 'raises NotImplementedError when performing validations' do + instance = test_class.new(source: target_issue, target: source_issue) + + expect { instance.save! }.to raise_error(NotImplementedError) + end + end + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 832d5b44e5d..e3c0e3a7a2b 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -18,7 +18,6 @@ RSpec.describe Issuable do it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:todos) } it { is_expected.to have_many(:labels) } - it { is_expected.to have_many(:note_authors).through(:notes) } context 'Notes' do let!(:note) { create(:note, noteable: issue, project: issue.project) } @@ -28,6 +27,23 @@ RSpec.describe Issuable do expect(issue.notes).not_to be_authors_loaded expect(scoped_issue.notes).to be_authors_loaded end + + describe 'note_authors' do + it { is_expected.to have_many(:note_authors).through(:notes) } + end + + describe 'user_note_authors' do + let_it_be(:system_user) { create(:user) } + + let!(:system_note) { create(:system_note, author: system_user, noteable: issue, project: issue.project) } + + it 'filters the authors to those of user notes' do + authors = issue.user_note_authors + + expect(authors).to include(note.author) + expect(authors).not_to include(system_user) + end + end end end @@ -572,6 +588,27 @@ RSpec.describe Issuable do issue.to_hook_data(user, old_associations: { severity: 'unknown' }) end end + + context 'escalation status is updated' do + let(:issue) { create(:incident, :with_escalation_status) } + let(:acknowledged) { IncidentManagement::IssuableEscalationStatus::STATUSES[:acknowledged] } + + before do + issue.escalation_status.update!(status: acknowledged) + + expect(Gitlab::HookData::IssuableBuilder).to receive(:new).with(issue).and_return(builder) + end + + it 'delegates to Gitlab::HookData::IssuableBuilder#build' do + expect(builder).to receive(:build).with( + user: user, + changes: hash_including( + 'escalation_status' => %i(triggered acknowledged) + )) + + issue.to_hook_data(user, old_associations: { escalation_status: :triggered }) + end + end end describe '#labels_array' do @@ -761,7 +798,7 @@ RSpec.describe Issuable do it 'updates issues updated_at' do issue - Timecop.travel(1.minute.from_now) do + travel_to(2.minutes.from_now) do expect { spend_time(1800) }.to change { issue.updated_at } end end @@ -786,7 +823,7 @@ RSpec.describe Issuable do context 'when time to subtract exceeds the total time spent' do it 'raise a validation error' do - Timecop.travel(1.minute.from_now) do + travel_to(1.minute.from_now) do expect do expect do spend_time(-3600) diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 3c095477ea9..9daea3438cb 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Mentionable do include Mentionable attr_accessor :project, :message + attr_mentionable :message def author diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb new file mode 100644 index 00000000000..db7f652f494 --- /dev/null +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -0,0 +1,177 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe PgFullTextSearchable do + let(:project) { create(:project) } + + let(:model_class) do + Class.new(ActiveRecord::Base) do + include PgFullTextSearchable + + self.table_name = 'issues' + + belongs_to :project + has_one :search_data, class_name: 'Issues::SearchData' + + def persist_pg_full_text_search_vector(search_vector) + Issues::SearchData.upsert({ project_id: project_id, issue_id: id, search_vector: search_vector }, unique_by: %i(project_id issue_id)) + end + + def self.name + 'Issue' + end + end + end + + describe '.pg_full_text_searchable' do + it 'sets pg_full_text_searchable_columns' do + model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }] + + expect(model_class.pg_full_text_searchable_columns).to eq({ 'title' => 'A' }) + end + + it 'raises an error when called twice' do + model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }] + + expect { model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }] }.to raise_error('Full text search columns already defined!') + end + end + + describe 'after commit hook' do + let(:model) { model_class.create!(project: project) } + + before do + model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }] + end + + context 'when specified columns are changed' do + it 'calls update_search_data!' do + expect(model).to receive(:update_search_data!) + + model.update!(title: 'A new title') + end + end + + context 'when specified columns are not changed' do + it 'does not enqueue worker' do + expect(model).not_to receive(:update_search_data!) + + model.update!(description: 'A new description') + end + end + end + + describe '.pg_full_text_search' do + let(:english) { model_class.create!(project: project, title: 'title', description: 'something english') } + let(:with_accent) { model_class.create!(project: project, title: 'Jürgen', description: 'Ærøskøbing') } + let(:japanese) { model_class.create!(project: project, title: '日本語 title', description: 'another english description') } + + before do + model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }, { name: 'description', weight: 'B' }] + + [english, with_accent, japanese].each(&:update_search_data!) + end + + it 'searches across all fields' do + expect(model_class.pg_full_text_search('title english')).to contain_exactly(english, japanese) + end + + it 'searches for exact term with quotes' do + expect(model_class.pg_full_text_search('"something english"')).to contain_exactly(english) + end + + it 'ignores accents' do + expect(model_class.pg_full_text_search('jurgen')).to contain_exactly(with_accent) + end + + it 'does not support searching by non-Latin characters' do + expect(model_class.pg_full_text_search('日本')).to be_empty + end + end + + describe '#update_search_data!' do + let(:model) { model_class.create!(project: project, title: 'title', description: 'description') } + + before do + model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }, { name: 'description', weight: 'B' }] + end + + it 'sets the correct weights' do + model.update_search_data! + + expect(model.search_data.search_vector).to match(/'titl':1A/) + expect(model.search_data.search_vector).to match(/'descript':2B/) + end + + context 'with accented and non-Latin characters' do + let(:model) { model_class.create!(project: project, title: '日本語', description: 'Jürgen') } + + it 'transliterates accented characters and removes non-Latin ones' do + model.update_search_data! + + expect(model.search_data.search_vector).not_to match(/日本語/) + expect(model.search_data.search_vector).to match(/jurgen/) + end + end + + context 'with long words' do + let(:model) { model_class.create!(project: project, title: 'title ' + 'long/sequence+1' * 4, description: 'description ' + '@user1' * 20) } + + it 'strips words that are 50 characters or longer' do + model.update_search_data! + + expect(model.search_data.search_vector).to match(/'titl':1A/) + expect(model.search_data.search_vector).not_to match(/long/) + expect(model.search_data.search_vector).not_to match(/sequence/) + + expect(model.search_data.search_vector).to match(/'descript':2B/) + expect(model.search_data.search_vector).not_to match(/@user1/) + end + end + + context 'when upsert times out' do + it 're-raises the exception' do + expect(Issues::SearchData).to receive(:upsert).once.and_raise(ActiveRecord::StatementTimeout) + + expect { model.update_search_data! }.to raise_error(ActiveRecord::StatementTimeout) + end + end + + context 'with strings that go over tsvector limit', :delete do + let(:long_string) { Array.new(30_000) { SecureRandom.hex }.join(' ') } + let(:model) { model_class.create!(project: project, title: 'title', description: long_string) } + + it 'does not raise an exception' do + expect(Gitlab::AppJsonLogger).to receive(:error).with( + a_hash_including(class: model_class.name, model_id: model.id) + ) + + expect { model.update_search_data! }.not_to raise_error + + expect(model.search_data).to eq(nil) + end + end + + context 'when model class does not implement persist_pg_full_text_search_vector' do + let(:model_class) do + Class.new(ActiveRecord::Base) do + include PgFullTextSearchable + + self.table_name = 'issues' + + belongs_to :project + has_one :search_data, class_name: 'Issues::SearchData' + + def self.name + 'Issue' + end + end + end + + it 'raises an error' do + expect { model.update_search_data! }.to raise_error(NotImplementedError) + end + end + end +end diff --git a/spec/models/concerns/runners_token_prefixable_spec.rb b/spec/models/concerns/runners_token_prefixable_spec.rb index 6127203987f..29e7b8cf4f4 100644 --- a/spec/models/concerns/runners_token_prefixable_spec.rb +++ b/spec/models/concerns/runners_token_prefixable_spec.rb @@ -3,18 +3,11 @@ require 'spec_helper' RSpec.describe RunnersTokenPrefixable do - before do - stub_const('DummyModel', Class.new) - DummyModel.class_eval do - include RunnersTokenPrefixable - end - end - - describe '.runners_token_prefix' do - subject { DummyModel.new } + describe 'runners token prefix' do + subject { described_class::RUNNERS_TOKEN_PREFIX } - it 'returns RUNNERS_TOKEN_PREFIX' do - expect(subject.runners_token_prefix).to eq(RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX) + it 'has the correct value' do + expect(subject).to eq('GR1348941') end end end diff --git a/spec/models/concerns/sensitive_serializable_hash_spec.rb b/spec/models/concerns/sensitive_serializable_hash_spec.rb new file mode 100644 index 00000000000..923f9e80c1f --- /dev/null +++ b/spec/models/concerns/sensitive_serializable_hash_spec.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SensitiveSerializableHash do + describe '.prevent_from_serialization' do + let(:test_class) do + Class.new do + include ActiveModel::Serialization + include SensitiveSerializableHash + + attr_accessor :name, :super_secret + + prevent_from_serialization :super_secret + + def attributes + { 'name' => nil, 'super_secret' => nil } + end + end + end + + let(:model) { test_class.new } + + it 'does not include the field in serializable_hash' do + expect(model.serializable_hash).not_to include('super_secret') + end + + context 'unsafe_serialization_hash option' do + it 'includes the field in serializable_hash' do + expect(model.serializable_hash(unsafe_serialization_hash: true)).to include('super_secret') + end + end + + context 'when prevent_sensitive_fields_from_serializable_hash feature flag is disabled' do + before do + stub_feature_flags(prevent_sensitive_fields_from_serializable_hash: false) + end + + it 'includes the field in serializable_hash' do + expect(model.serializable_hash).to include('super_secret') + end + end + end + + describe '#serializable_hash' do + shared_examples "attr_encrypted attribute" do |klass, attribute_name| + context "#{klass.name}\##{attribute_name}" do + let(:attributes) { [attribute_name, "encrypted_#{attribute_name}", "encrypted_#{attribute_name}_iv"] } + + it 'has a encrypted_attributes field' do + expect(klass.encrypted_attributes).to include(attribute_name.to_sym) + end + + it 'does not include the attribute in serializable_hash', :aggregate_failures do + attributes.each do |attribute| + expect(model.attributes).to include(attribute) # double-check the attribute does exist + + expect(model.serializable_hash).not_to include(attribute) + expect(model.to_json).not_to include(attribute) + expect(model.as_json).not_to include(attribute) + end + end + + context 'unsafe_serialization_hash option' do + it 'includes the field in serializable_hash' do + attributes.each do |attribute| + expect(model.attributes).to include(attribute) # double-check the attribute does exist + + expect(model.serializable_hash(unsafe_serialization_hash: true)).to include(attribute) + expect(model.to_json(unsafe_serialization_hash: true)).to include(attribute) + expect(model.as_json(unsafe_serialization_hash: true)).to include(attribute) + end + end + end + end + end + + it_behaves_like 'attr_encrypted attribute', WebHook, 'token' do + let_it_be(:model) { create(:system_hook) } + end + + it_behaves_like 'attr_encrypted attribute', Ci::InstanceVariable, 'value' do + let_it_be(:model) { create(:ci_instance_variable) } + end + + shared_examples "add_authentication_token_field attribute" do |klass, attribute_name, encrypted_attribute: true, digest_attribute: false| + context "#{klass.name}\##{attribute_name}" do + let(:attributes) do + if digest_attribute + ["#{attribute_name}_digest"] + elsif encrypted_attribute + [attribute_name, "#{attribute_name}_encrypted"] + else + [attribute_name] + end + end + + it 'has a add_authentication_token_field field' do + expect(klass.token_authenticatable_fields).to include(attribute_name.to_sym) + end + + it 'does not include the attribute in serializable_hash', :aggregate_failures do + attributes.each do |attribute| + expect(model.attributes).to include(attribute) # double-check the attribute does exist + + expect(model.serializable_hash).not_to include(attribute) + expect(model.to_json).not_to include(attribute) + expect(model.as_json).not_to include(attribute) + end + end + + context 'unsafe_serialization_hash option' do + it 'includes the field in serializable_hash' do + attributes.each do |attribute| + expect(model.attributes).to include(attribute) # double-check the attribute does exist + + expect(model.serializable_hash(unsafe_serialization_hash: true)).to include(attribute) + expect(model.to_json(unsafe_serialization_hash: true)).to include(attribute) + expect(model.as_json(unsafe_serialization_hash: true)).to include(attribute) + end + end + end + end + end + + it_behaves_like 'add_authentication_token_field attribute', Ci::Runner, 'token' do + let_it_be(:model) { create(:ci_runner) } + + it 'does not include token_expires_at in serializable_hash' do + attribute = 'token_expires_at' + + expect(model.attributes).to include(attribute) # double-check the attribute does exist + + expect(model.serializable_hash).not_to include(attribute) + expect(model.to_json).not_to include(attribute) + expect(model.as_json).not_to include(attribute) + end + end + + it_behaves_like 'add_authentication_token_field attribute', ApplicationSetting, 'health_check_access_token', encrypted_attribute: false do + # health_check_access_token_encrypted column does not exist + let_it_be(:model) { create(:application_setting) } + end + + it_behaves_like 'add_authentication_token_field attribute', PersonalAccessToken, 'token', encrypted_attribute: false, digest_attribute: true do + # PersonalAccessToken only has token_digest column + let_it_be(:model) { create(:personal_access_token) } + end + end +end diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 5edaab56e2d..baa2d75705a 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -55,7 +55,7 @@ RSpec.describe Spammable do subject { invalidate_if_spam(needs_recaptcha: true) } it 'has an error related to spam on the model' do - expect(subject.errors.messages[:base]).to match_array /solve the reCAPTCHA/ + expect(subject.errors.messages[:base]).to match_array /content or solve the/ end end @@ -63,7 +63,7 @@ RSpec.describe Spammable do subject { invalidate_if_spam(is_spam: true, needs_recaptcha: true) } it 'has an error related to spam on the model' do - expect(subject.errors.messages[:base]).to match_array /solve the reCAPTCHA/ + expect(subject.errors.messages[:base]).to match_array /content or solve the/ end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 4534fd3664e..d7bfcc3f579 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -9,6 +9,12 @@ RSpec.shared_examples 'TokenAuthenticatable' do it { is_expected.to respond_to("set_#{token_field}") } it { is_expected.to respond_to("reset_#{token_field}!") } end + + describe 'SensitiveSerializableHash' do + it 'includes the token field in list of sensitive attributes prevented from serialization' do + expect(described_class.attributes_exempt_from_serializable_hash).to include(token_field) + end + end end RSpec.describe User, 'TokenAuthenticatable' do diff --git a/spec/models/concerns/token_authenticatable_strategies/base_spec.rb b/spec/models/concerns/token_authenticatable_strategies/base_spec.rb index bccef9b9554..89ddc797a9d 100644 --- a/spec/models/concerns/token_authenticatable_strategies/base_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/base_spec.rb @@ -6,6 +6,24 @@ RSpec.describe TokenAuthenticatableStrategies::Base do let(:instance) { double(:instance) } let(:field) { double(:field) } + describe '#token_fields' do + let(:strategy) { described_class.new(instance, field, options) } + let(:field) { 'some_token' } + let(:options) { {} } + + it 'includes the token field' do + expect(strategy.token_fields).to contain_exactly(field) + end + + context 'with expires_at option' do + let(:options) { { expires_at: true } } + + it 'includes the token_expires_at field' do + expect(strategy.token_fields).to contain_exactly(field, 'some_token_expires_at') + end + end + end + describe '.fabricate' do context 'when digest stragegy is specified' do it 'fabricates digest strategy object' do diff --git a/spec/models/concerns/token_authenticatable_strategies/digest_spec.rb b/spec/models/concerns/token_authenticatable_strategies/digest_spec.rb new file mode 100644 index 00000000000..bcd6e1e7316 --- /dev/null +++ b/spec/models/concerns/token_authenticatable_strategies/digest_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe TokenAuthenticatableStrategies::Digest do + let(:model) { class_double('Project') } + let(:options) { { digest: true } } + + subject(:strategy) do + described_class.new(model, 'some_field', options) + end + + describe '#token_fields' do + it 'includes the digest field' do + expect(strategy.token_fields).to contain_exactly('some_field', 'some_field_digest') + end + end +end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index 458dfb47394..e0ebb86585a 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -14,10 +14,18 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value') end - subject do + subject(:strategy) do described_class.new(model, 'some_field', options) end + describe '#token_fields' do + let(:options) { { encrypted: :required } } + + it 'includes the encrypted field' do + expect(strategy.token_fields).to contain_exactly('some_field', 'some_field_encrypted') + end + end + describe '#find_token_authenticatable' do context 'when encryption is required' do let(:options) { { encrypted: :required } } diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 7c0ae51223b..c8d86edc55f 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -653,6 +653,58 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end + describe '#size' do + let(:on_com) { true } + let(:created_at) { described_class::MIGRATION_PHASE_1_STARTED_AT + 3.months } + + subject { repository.size } + + before do + allow(::Gitlab).to receive(:com?).and_return(on_com) + allow(repository).to receive(:created_at).and_return(created_at) + end + + context 'supports gitlab api on .com with a recent repository' do + before do + expect(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(true) + expect(repository.gitlab_api_client).to receive(:repository_details).with(repository.path, with_size: true).and_return(response) + end + + context 'with a size_bytes field' do + let(:response) { { 'size_bytes' => 12345 } } + + it { is_expected.to eq(12345) } + end + + context 'without a size_bytes field' do + let(:response) { { 'foo' => 'bar' } } + + it { is_expected.to eq(nil) } + end + end + + context 'does not support gitlab api' do + before do + expect(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(false) + expect(repository.gitlab_api_client).not_to receive(:repository_details) + end + + it { is_expected.to eq(nil) } + end + + context 'not on .com' do + let(:on_com) { false } + + it { is_expected.to eq(nil) } + end + + context 'with an old repository' do + let(:created_at) { described_class::MIGRATION_PHASE_1_STARTED_AT - 3.months } + + it { is_expected.to eq(nil) } + end + end + describe '#reset_expiration_policy_started_at!' do subject { repository.reset_expiration_policy_started_at! } @@ -1203,7 +1255,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do subject { described_class.ready_for_import } before do - stub_application_setting(container_registry_import_target_plan: project.namespace.actual_plan_name) + stub_application_setting(container_registry_import_target_plan: root_group.actual_plan_name) end it 'works' do diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb index c7b0f1bd3d4..18896962261 100644 --- a/spec/models/customer_relations/contact_spec.rb +++ b/spec/models/customer_relations/contact_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe CustomerRelations::Contact, type: :model do + let_it_be(:group) { create(:group) } + describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:organization).optional } @@ -23,6 +25,8 @@ RSpec.describe CustomerRelations::Contact, type: :model do it { is_expected.to validate_length_of(:email).is_at_most(255) } it { is_expected.to validate_length_of(:description).is_at_most(1024) } + it { is_expected.to validate_uniqueness_of(:email).scoped_to(:group_id) } + it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email end @@ -38,33 +42,15 @@ RSpec.describe CustomerRelations::Contact, type: :model do it { expect(described_class.reference_postfix).to eq(']') } end - describe '#unique_email_for_group_hierarchy' do - let_it_be(:parent) { create(:group) } - let_it_be(:group) { create(:group, parent: parent) } - let_it_be(:subgroup) { create(:group, parent: group) } - - let_it_be(:existing_contact) { create(:contact, group: group) } - - context 'with unique email for group hierarchy' do + describe '#root_group' do + context 'when root group' do subject { build(:contact, group: group) } it { is_expected.to be_valid } end - context 'with duplicate email in group' do - subject { build(:contact, email: existing_contact.email, group: group) } - - it { is_expected.to be_invalid } - end - - context 'with duplicate email in parent group' do - subject { build(:contact, email: existing_contact.email, group: subgroup) } - - it { is_expected.to be_invalid } - end - - context 'with duplicate email in subgroup' do - subject { build(:contact, email: existing_contact.email, group: parent) } + context 'when subgroup' do + subject { build(:contact, group: create(:group, parent: group)) } it { is_expected.to be_invalid } end @@ -82,7 +68,6 @@ RSpec.describe CustomerRelations::Contact, type: :model do end describe '#self.find_ids_by_emails' do - let_it_be(:group) { create(:group) } let_it_be(:group_contacts) { create_list(:contact, 2, group: group) } let_it_be(:other_contacts) { create_list(:contact, 2) } @@ -92,13 +77,6 @@ RSpec.describe CustomerRelations::Contact, type: :model do expect(contact_ids).to match_array(group_contacts.pluck(:id)) end - it 'returns ids of contacts from parent group' do - subgroup = create(:group, parent: group) - contact_ids = described_class.find_ids_by_emails(subgroup, group_contacts.pluck(:email)) - - expect(contact_ids).to match_array(group_contacts.pluck(:id)) - end - it 'does not return ids of contacts from other groups' do contact_ids = described_class.find_ids_by_emails(group, other_contacts.pluck(:email)) @@ -112,28 +90,17 @@ RSpec.describe CustomerRelations::Contact, type: :model do end describe '#self.exists_for_group?' do - let(:group) { create(:group) } - let(:subgroup) { create(:group, parent: group) } - - context 'with no contacts in group or parent' do + context 'with no contacts in group' do it 'returns false' do - expect(described_class.exists_for_group?(subgroup)).to be_falsey + expect(described_class.exists_for_group?(group)).to be_falsey end end context 'with contacts in group' do it 'returns true' do - create(:contact, group: subgroup) - - expect(described_class.exists_for_group?(subgroup)).to be_truthy - end - end - - context 'with contacts in parent' do - it 'returns true' do create(:contact, group: group) - expect(described_class.exists_for_group?(subgroup)).to be_truthy + expect(described_class.exists_for_group?(group)).to be_truthy end end end diff --git a/spec/models/customer_relations/issue_contact_spec.rb b/spec/models/customer_relations/issue_contact_spec.rb index 39da0b64ea0..f1fb574f86f 100644 --- a/spec/models/customer_relations/issue_contact_spec.rb +++ b/spec/models/customer_relations/issue_contact_spec.rb @@ -6,7 +6,8 @@ RSpec.describe CustomerRelations::IssueContact do let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) } let_it_be(:group) { create(:group) } let_it_be(:subgroup) { create(:group, parent: group) } - let_it_be(:project) { create(:project, group: subgroup) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:subgroup_project) { create(:project, group: subgroup) } let_it_be(:issue) { create(:issue, project: project) } subject { issue_contact } @@ -27,33 +28,36 @@ RSpec.describe CustomerRelations::IssueContact do let(:for_issue) { build(:issue_customer_relations_contact, :for_issue, issue: issue) } let(:for_contact) { build(:issue_customer_relations_contact, :for_contact, contact: contact) } - it 'uses objects from the same group', :aggregate_failures do - expect(stubbed.contact.group).to eq(stubbed.issue.project.group) - expect(built.contact.group).to eq(built.issue.project.group) - expect(created.contact.group).to eq(created.issue.project.group) + context 'for root groups' do + it 'uses objects from the same group', :aggregate_failures do + expect(stubbed.contact.group).to eq(stubbed.issue.project.group) + expect(built.contact.group).to eq(built.issue.project.group) + expect(created.contact.group).to eq(created.issue.project.group) + end end - it 'builds using the same group', :aggregate_failures do - expect(for_issue.contact.group).to eq(subgroup) - expect(for_contact.issue.project.group).to eq(group) + context 'for subgroups' do + it 'builds using the root ancestor' do + expect(for_issue.contact.group).to eq(group) + end end end describe 'validation' do - it 'fails when the contact group does not belong to the issue group or ancestors' do + it 'fails when the contact group is unrelated to the issue group' do built = build(:issue_customer_relations_contact, issue: create(:issue), contact: create(:contact)) expect(built).not_to be_valid end - it 'succeeds when the contact group is the same as the issue group' do - built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: subgroup)) + it 'succeeds when the contact belongs to a root group and is the same as the issue group' do + built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: group)) expect(built).to be_valid end - it 'succeeds when the contact group is an ancestor of the issue group' do - built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: group)) + it 'succeeds when the contact belongs to a root group and it is an ancestor of the issue group' do + built = build(:issue_customer_relations_contact, issue: create(:issue, project: subgroup_project), contact: create(:contact, group: group)) expect(built).to be_valid end diff --git a/spec/models/customer_relations/organization_spec.rb b/spec/models/customer_relations/organization_spec.rb index 71b455ae8c8..9fe754b7605 100644 --- a/spec/models/customer_relations/organization_spec.rb +++ b/spec/models/customer_relations/organization_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe CustomerRelations::Organization, type: :model do + let_it_be(:group) { create(:group) } + describe 'associations' do it { is_expected.to belong_to(:group).with_foreign_key('group_id') } end @@ -17,6 +19,20 @@ RSpec.describe CustomerRelations::Organization, type: :model do it { is_expected.to validate_length_of(:description).is_at_most(1024) } end + describe '#root_group' do + context 'when root group' do + subject { build(:organization, group: group) } + + it { is_expected.to be_valid } + end + + context 'when subgroup' do + subject { build(:organization, group: create(:group, parent: group)) } + + it { is_expected.to be_invalid } + end + end + describe '#name' do it 'strips name' do organization = described_class.new(name: ' GitLab ') @@ -27,7 +43,6 @@ RSpec.describe CustomerRelations::Organization, type: :model do end describe '#find_by_name' do - let!(:group) { create(:group) } let!(:organiztion1) { create(:organization, group: group, name: 'Test') } let!(:organiztion2) { create(:organization, group: create(:group), name: 'Test') } diff --git a/spec/models/dependency_proxy/blob_spec.rb b/spec/models/dependency_proxy/blob_spec.rb index 10d06406ad7..cc62aecd1ab 100644 --- a/spec/models/dependency_proxy/blob_spec.rb +++ b/spec/models/dependency_proxy/blob_spec.rb @@ -5,6 +5,10 @@ RSpec.describe DependencyProxy::Blob, type: :model do it_behaves_like 'ttl_expirable' it_behaves_like 'destructible', factory: :dependency_proxy_blob + it_behaves_like 'updates namespace statistics' do + let(:statistic_source) { build(:dependency_proxy_blob, size: 10) } + end + describe 'relationships' do it { is_expected.to belong_to(:group) } end diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index ab7881b1d39..d43079f607a 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -5,6 +5,10 @@ RSpec.describe DependencyProxy::Manifest, type: :model do it_behaves_like 'ttl_expirable' it_behaves_like 'destructible', factory: :dependency_proxy_manifest + it_behaves_like 'updates namespace statistics' do + let(:statistic_source) { build(:dependency_proxy_manifest, size: 10) } + end + describe 'relationships' do it { is_expected.to belong_to(:group) } end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 112dc93658f..6144593395c 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -282,6 +282,13 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do 'DEV' | described_class.tiers[:development] 'development' | described_class.tiers[:development] 'trunk' | described_class.tiers[:development] + 'dev' | described_class.tiers[:development] + 'review/app' | described_class.tiers[:development] + 'PRODUCTION' | described_class.tiers[:production] + 'prod' | described_class.tiers[:production] + 'prod-east-2' | described_class.tiers[:production] + 'us-prod-east' | described_class.tiers[:production] + 'fe-production' | described_class.tiers[:production] 'test' | described_class.tiers[:testing] 'TEST' | described_class.tiers[:testing] 'testing' | described_class.tiers[:testing] @@ -290,6 +297,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do 'production-test' | described_class.tiers[:testing] 'test-production' | described_class.tiers[:testing] 'QC' | described_class.tiers[:testing] + 'qa-env-2' | described_class.tiers[:testing] 'gstg' | described_class.tiers[:staging] 'staging' | described_class.tiers[:staging] 'stage' | described_class.tiers[:staging] @@ -298,6 +306,10 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do 'Pre-production' | described_class.tiers[:staging] 'pre' | described_class.tiers[:staging] 'Demo' | described_class.tiers[:staging] + 'staging' | described_class.tiers[:staging] + 'pre-prod' | described_class.tiers[:staging] + 'blue-kit-stage' | described_class.tiers[:staging] + 'pre-prod' | described_class.tiers[:staging] 'gprd' | described_class.tiers[:production] 'gprd-cny' | described_class.tiers[:production] 'production' | described_class.tiers[:production] @@ -307,6 +319,8 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do 'production/eu' | described_class.tiers[:production] 'PRODUCTION/EU' | described_class.tiers[:production] 'productioneu' | described_class.tiers[:production] + 'store-produce' | described_class.tiers[:production] + 'unproductive' | described_class.tiers[:production] 'production/www.gitlab.com' | described_class.tiers[:production] 'prod' | described_class.tiers[:production] 'PROD' | described_class.tiers[:production] @@ -314,6 +328,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do 'canary' | described_class.tiers[:other] 'other' | described_class.tiers[:other] 'EXP' | described_class.tiers[:other] + 'something-else' | described_class.tiers[:other] end with_them do diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index d17541b4a6c..d700eb5eaf7 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -535,6 +535,25 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end + describe '#integrated_enabled?' do + using RSpec::Parameterized::TableSyntax + + where(:enabled, :integrated, :integrated_enabled) do + true | false | false + false | true | false + true | true | true + end + + with_them do + before do + subject.enabled = enabled + subject.integrated = integrated + end + + it { expect(subject.integrated_enabled?).to eq(integrated_enabled) } + end + end + describe '#gitlab_dsn' do let!(:client_key) { create(:error_tracking_client_key, project: project) } diff --git a/spec/models/event_collection_spec.rb b/spec/models/event_collection_spec.rb index 107447c9630..036072aab76 100644 --- a/spec/models/event_collection_spec.rb +++ b/spec/models/event_collection_spec.rb @@ -71,9 +71,9 @@ RSpec.describe EventCollection do end it 'can paginate through events' do - events = described_class.new(projects, offset: 20).to_a + events = described_class.new(projects, limit: 5, offset: 15).to_a - expect(events.length).to eq(2) + expect(events.length).to eq(5) end it 'returns an empty Array when crossing the maximum page number' do @@ -124,6 +124,19 @@ RSpec.describe EventCollection do expect(subject).to eq([event1]) end + + context 'pagination through events' do + let_it_be(:project_events) { create_list(:event, 10, project: project) } + let_it_be(:group_events) { create_list(:event, 10, group: group, author: user) } + + let(:subject) { described_class.new(projects, limit: 10, offset: 5, groups: groups).to_a } + + it 'returns recent groups and projects events' do + recent_events_with_offset = (project_events[5..] + group_events[..4]).reverse + + expect(subject).to eq(recent_events_with_offset) + end + end end end end diff --git a/spec/models/external_pull_request_spec.rb b/spec/models/external_pull_request_spec.rb index 82da7cdf34b..10136dd0bdb 100644 --- a/spec/models/external_pull_request_spec.rb +++ b/spec/models/external_pull_request_spec.rb @@ -233,10 +233,6 @@ RSpec.describe ExternalPullRequest do end end - it_behaves_like 'it has loose foreign keys' do - let(:factory_name) { :external_pull_request } - end - context 'loose foreign key on external_pull_requests.project_id' do it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index b6c7d61a291..45a2c134077 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -385,23 +385,25 @@ RSpec.describe Group do end end - before do - subject - reload_models(old_parent, new_parent, group) - end - context 'within the same hierarchy' do let!(:root) { create(:group).reload } let!(:old_parent) { create(:group, parent: root) } let!(:new_parent) { create(:group, parent: root) } - it 'updates traversal_ids' do - expect(group.traversal_ids).to eq [root.id, new_parent.id, group.id] - end + context 'with FOR NO KEY UPDATE lock' do + before do + subject + reload_models(old_parent, new_parent, group) + end - it_behaves_like 'hierarchy with traversal_ids' - it_behaves_like 'locked row' do - let(:row) { root } + it 'updates traversal_ids' do + expect(group.traversal_ids).to eq [root.id, new_parent.id, group.id] + end + + it_behaves_like 'hierarchy with traversal_ids' + it_behaves_like 'locked row' do + let(:row) { root } + end end end @@ -410,6 +412,11 @@ RSpec.describe Group do let!(:new_parent) { create(:group) } let!(:group) { create(:group, parent: old_parent) } + before do + subject + reload_models(old_parent, new_parent, group) + end + it 'updates traversal_ids' do expect(group.traversal_ids).to eq [new_parent.id, group.id] end @@ -435,6 +442,11 @@ RSpec.describe Group do let!(:old_parent) { nil } let!(:new_parent) { create(:group) } + before do + subject + reload_models(old_parent, new_parent, group) + end + it 'updates traversal_ids' do expect(group.traversal_ids).to eq [new_parent.id, group.id] end @@ -452,6 +464,11 @@ RSpec.describe Group do let!(:old_parent) { create(:group) } let!(:new_parent) { nil } + before do + subject + reload_models(old_parent, new_parent, group) + end + it 'updates traversal_ids' do expect(group.traversal_ids).to eq [group.id] end @@ -1327,10 +1344,14 @@ RSpec.describe Group do let!(:group) { create(:group, :nested) } let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let!(:pending_maintainer) { create(:group_member, :awaiting, :maintainer, group: group.parent) } + let!(:pending_developer) { create(:group_member, :awaiting, :developer, group: group) } - it 'returns parents members' do + it 'returns parents active members' do expect(group.members_with_parents).to include(developer) expect(group.members_with_parents).to include(maintainer) + expect(group.members_with_parents).not_to include(pending_developer) + expect(group.members_with_parents).not_to include(pending_maintainer) end context 'group sharing' do @@ -1340,9 +1361,11 @@ RSpec.describe Group do create(:group_group_link, shared_group: shared_group, shared_with_group: group) end - it 'returns shared with group members' do + it 'returns shared with group active members' do expect(shared_group.members_with_parents).to( include(developer)) + expect(shared_group.members_with_parents).not_to( + include(pending_developer)) end end end @@ -2168,7 +2191,7 @@ RSpec.describe Group do let(:group) { create(:group) } - subject { group.first_auto_devops_config } + subject(:fetch_config) { group.first_auto_devops_config } where(:instance_value, :group_value, :config) do # Instance level enabled @@ -2193,6 +2216,8 @@ RSpec.describe Group do end context 'with parent groups' do + let(:parent) { create(:group) } + where(:instance_value, :parent_value, :group_value, :config) do # Instance level enabled true | nil | nil | { status: true, scope: :instance } @@ -2222,17 +2247,82 @@ RSpec.describe Group do end with_them do + def define_cache_expectations(cache_key) + if group_value.nil? + expect(Rails.cache).to receive(:fetch).with(start_with(cache_key), expires_in: 1.day) + else + expect(Rails.cache).not_to receive(:fetch).with(start_with(cache_key), expires_in: 1.day) + end + end + before do stub_application_setting(auto_devops_enabled: instance_value) - parent = create(:group, auto_devops_enabled: parent_value) group.update!( auto_devops_enabled: group_value, parent: parent ) + parent.update!(auto_devops_enabled: parent_value) + + group.reload # Reload so we get the populated traversal IDs end it { is_expected.to eq(config) } + + it 'caches the parent config when group auto_devops_enabled is nil' do + cache_key = "namespaces:{#{group.traversal_ids.first}}:first_auto_devops_config:#{group.id}" + define_cache_expectations(cache_key) + + fetch_config + end + + context 'when traversal ID feature flags are disabled' do + before do + stub_feature_flags(sync_traversal_ids: false) + end + + it 'caches the parent config when group auto_devops_enabled is nil' do + cache_key = "namespaces:{first_auto_devops_config}:#{group.id}" + define_cache_expectations(cache_key) + + fetch_config + end + end + end + + context 'cache expiration' do + before do + group.update!(parent: parent) + + reload_models(parent) + end + + it 'clears both self and descendant cache when the parent value is updated' do + expect(Rails.cache).to receive(:delete_multi) + .with( + match_array([ + start_with("namespaces:{#{parent.traversal_ids.first}}:first_auto_devops_config:#{parent.id}"), + start_with("namespaces:{#{parent.traversal_ids.first}}:first_auto_devops_config:#{group.id}") + ]) + ) + + parent.update!(auto_devops_enabled: true) + end + + it 'only clears self cache when there are no dependents' do + expect(Rails.cache).to receive(:delete_multi) + .with([start_with("namespaces:{#{group.traversal_ids.first}}:first_auto_devops_config:#{group.id}")]) + + group.update!(auto_devops_enabled: true) + end + + it 'does not clear cache when the feature is disabled' do + stub_feature_flags(namespaces_cache_first_auto_devops_config: false) + + expect(Rails.cache).not_to receive(:delete_multi) + + parent.update!(auto_devops_enabled: true) + end end end end @@ -2860,7 +2950,14 @@ RSpec.describe Group do expect(group.crm_enabled?).to be_truthy end + + it 'returns true where crm_settings.state is enabled for subgroup' do + subgroup = create(:group, :crm_enabled, parent: group) + + expect(subgroup.crm_enabled?).to be_truthy + end end + describe '.get_ids_by_ids_or_paths' do let(:group_path) { 'group_path' } let!(:group) { create(:group, path: group_path) } @@ -3149,12 +3246,4 @@ RSpec.describe Group do it_behaves_like 'no effective expiration interval' end end - - describe '#runners_token' do - let_it_be(:group) { create(:group) } - - subject { group } - - it_behaves_like 'it has a prefixable runners_token' - end end diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index 8dd9cf9e84a..9cfbb14e087 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -15,7 +15,7 @@ RSpec.describe WebHookLog do let(:hook) { create(:project_hook) } it 'does not return web hook logs that are too old' do - create(:web_hook_log, web_hook: hook, created_at: 91.days.ago) + create(:web_hook_log, web_hook: hook, created_at: 10.days.ago) expect(described_class.recent.size).to be_zero end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 482e372543c..dd954e08156 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -432,6 +432,12 @@ RSpec.describe WebHook do expect(hook).not_to be_temporarily_disabled end + + it 'can ignore the feature flag' do + stub_feature_flags(web_hooks_disable_failed: false) + + expect(hook).to be_temporarily_disabled(ignore_flag: true) + end end end @@ -454,6 +460,12 @@ RSpec.describe WebHook do expect(hook).not_to be_permanently_disabled end + + it 'can ignore the feature flag' do + stub_feature_flags(web_hooks_disable_failed: false) + + expect(hook).to be_permanently_disabled(ignore_flag: true) + end end end diff --git a/spec/models/incident_management/issuable_escalation_status_spec.rb b/spec/models/incident_management/issuable_escalation_status_spec.rb index c548357bd3f..f956be3a04e 100644 --- a/spec/models/incident_management/issuable_escalation_status_spec.rb +++ b/spec/models/incident_management/issuable_escalation_status_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe IncidentManagement::IssuableEscalationStatus do - let_it_be(:issue) { create(:issue) } + let_it_be(:issue) { create(:incident) } subject(:escalation_status) { build(:incident_management_issuable_escalation_status, issue: issue) } diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 6b0d8d7ca4a..3af717798c3 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -206,7 +206,8 @@ RSpec.describe InstanceConfiguration do group_download_export_limit: 1019, group_import_limit: 1020, raw_blob_request_limit: 1021, - user_email_lookup_limit: 1022, + search_rate_limit: 1022, + search_rate_limit_unauthenticated: 1000, users_get_by_id_limit: 1023 ) end @@ -230,7 +231,8 @@ RSpec.describe InstanceConfiguration do expect(rate_limits[:group_export_download]).to eq({ enabled: true, requests_per_period: 1019, period_in_seconds: 60 }) expect(rate_limits[:group_import]).to eq({ enabled: true, requests_per_period: 1020, period_in_seconds: 60 }) expect(rate_limits[:raw_blob]).to eq({ enabled: true, requests_per_period: 1021, period_in_seconds: 60 }) - expect(rate_limits[:user_email_lookup]).to eq({ enabled: true, requests_per_period: 1022, period_in_seconds: 60 }) + expect(rate_limits[:search_rate_limit]).to eq({ enabled: true, requests_per_period: 1022, period_in_seconds: 60 }) + expect(rate_limits[:search_rate_limit_unauthenticated]).to eq({ enabled: true, requests_per_period: 1000, period_in_seconds: 60 }) expect(rate_limits[:users_get_by_id]).to eq({ enabled: true, requests_per_period: 1023, period_in_seconds: 600 }) end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index e822620ab80..48d8ba975b6 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -85,14 +85,14 @@ RSpec.describe Integration do subject { described_class.by_type(type) } - context 'when type is "JiraService"' do - let(:type) { 'JiraService' } + context 'when type is "Integrations::JiraService"' do + let(:type) { 'Integrations::Jira' } it { is_expected.to match_array([integration1, integration2]) } end - context 'when type is "RedmineService"' do - let(:type) { 'RedmineService' } + context 'when type is "Integrations::Redmine"' do + let(:type) { 'Integrations::Redmine' } it { is_expected.to match_array([integration3]) } end @@ -103,7 +103,7 @@ RSpec.describe Integration do let!(:integration2) { create(:jira_integration) } it 'returns the right group integration' do - expect(described_class.for_group(group)).to match_array([integration1]) + expect(described_class.for_group(group)).to contain_exactly(integration1) end end @@ -268,7 +268,7 @@ RSpec.describe Integration do describe '.build_from_integration' do context 'when integration is invalid' do let(:invalid_integration) do - build(:prometheus_integration, :template, active: true, properties: {}) + build(:prometheus_integration, :instance, active: true, properties: {}) .tap { |integration| integration.save!(validate: false) } end @@ -376,22 +376,24 @@ RSpec.describe Integration do let_it_be(:instance_integration) { create(:jira_integration, :instance) } it 'returns the instance integration' do - expect(described_class.default_integration('JiraService', project)).to eq(instance_integration) + expect(described_class.default_integration('Integrations::Jira', project)).to eq(instance_integration) end it 'returns nil for nonexistent integration type' do - expect(described_class.default_integration('HipchatService', project)).to eq(nil) + expect(described_class.default_integration('Integrations::Hipchat', project)).to eq(nil) end context 'with a group integration' do + let(:integration_name) { 'Integrations::Jira' } + let_it_be(:group_integration) { create(:jira_integration, group_id: group.id, project_id: nil) } it 'returns the group integration for a project' do - expect(described_class.default_integration('JiraService', project)).to eq(group_integration) + expect(described_class.default_integration(integration_name, project)).to eq(group_integration) end it 'returns the instance integration for a group' do - expect(described_class.default_integration('JiraService', group)).to eq(instance_integration) + expect(described_class.default_integration(integration_name, group)).to eq(instance_integration) end context 'with a subgroup' do @@ -400,18 +402,18 @@ RSpec.describe Integration do let!(:project) { create(:project, group: subgroup) } it 'returns the closest group integration for a project' do - expect(described_class.default_integration('JiraService', project)).to eq(group_integration) + expect(described_class.default_integration(integration_name, project)).to eq(group_integration) end it 'returns the closest group integration for a subgroup' do - expect(described_class.default_integration('JiraService', subgroup)).to eq(group_integration) + expect(described_class.default_integration(integration_name, subgroup)).to eq(group_integration) end context 'having a integration with custom settings' do let!(:subgroup_integration) { create(:jira_integration, group_id: subgroup.id, project_id: nil) } it 'returns the closest group integration for a project' do - expect(described_class.default_integration('JiraService', project)).to eq(subgroup_integration) + expect(described_class.default_integration(integration_name, project)).to eq(subgroup_integration) end end @@ -419,7 +421,7 @@ RSpec.describe Integration do let!(:subgroup_integration) { create(:jira_integration, group_id: subgroup.id, project_id: nil, inherit_from_id: group_integration.id) } it 'returns the closest group integration which does not inherit from its parent for a project' do - expect(described_class.default_integration('JiraService', project)).to eq(group_integration) + expect(described_class.default_integration(integration_name, project)).to eq(group_integration) end end end @@ -556,13 +558,26 @@ RSpec.describe Integration do end end - describe '.integration_name_to_model' do - it 'returns the model for the given integration name' do - expect(described_class.integration_name_to_model('asana')).to eq(Integrations::Asana) + describe '.integration_name_to_type' do + it 'handles a simple case' do + expect(described_class.integration_name_to_type(:asana)).to eq 'Integrations::Asana' + end + + it 'raises an error if the name is unknown' do + expect { described_class.integration_name_to_type('foo') } + .to raise_exception(described_class::UnknownType, /foo/) + end + + it 'handles all available_integration_names' do + types = described_class.available_integration_names.map { described_class.integration_name_to_type(_1) } + + expect(types).to all(start_with('Integrations::')) end + end + describe '.integration_name_to_model' do it 'raises an error if integration name is invalid' do - expect { described_class.integration_name_to_model('foo') }.to raise_exception(NameError, /uninitialized constant FooService/) + expect { described_class.integration_name_to_model('foo') }.to raise_exception(described_class::UnknownType, /foo/) end end @@ -704,27 +719,63 @@ RSpec.describe Integration do end describe '#api_field_names' do - let(:fake_integration) do - Class.new(Integration) do - def fields - [ - { name: 'token' }, - { name: 'api_token' }, - { name: 'token_api' }, - { name: 'safe_token' }, - { name: 'key' }, - { name: 'api_key' }, - { name: 'password' }, - { name: 'password_field' }, - { name: 'some_safe_field' }, - { name: 'safe_field' } - ].shuffle - end + shared_examples 'api field names' do + it 'filters out sensitive fields' do + safe_fields = %w[some_safe_field safe_field url trojan_gift] + + expect(fake_integration.new).to have_attributes( + api_field_names: match_array(safe_fields) + ) end end - it 'filters out sensitive fields' do - expect(fake_integration.new).to have_attributes(api_field_names: match_array(%w[some_safe_field safe_field])) + context 'when the class overrides #fields' do + let(:fake_integration) do + Class.new(Integration) do + def fields + [ + { name: 'token' }, + { name: 'api_token' }, + { name: 'token_api' }, + { name: 'safe_token' }, + { name: 'key' }, + { name: 'api_key' }, + { name: 'password' }, + { name: 'password_field' }, + { name: 'some_safe_field' }, + { name: 'safe_field' }, + { name: 'url' }, + { name: 'trojan_horse', type: 'password' }, + { name: 'trojan_gift', type: 'gift' } + ].shuffle + end + end + end + + it_behaves_like 'api field names' + end + + context 'when the class uses the field DSL' do + let(:fake_integration) do + Class.new(described_class) do + field :token + field :token + field :api_token + field :token_api + field :safe_token + field :key + field :api_key + field :password + field :password_field + field :some_safe_field + field :safe_field + field :url + field :trojan_horse, type: 'password' + field :trojan_gift, type: 'gift' + end + end + + it_behaves_like 'api field names' end end @@ -774,35 +825,33 @@ RSpec.describe Integration do end describe '.available_integration_names' do - it 'calls the right methods' do - expect(described_class).to receive(:integration_names).and_call_original - expect(described_class).to receive(:dev_integration_names).and_call_original - expect(described_class).to receive(:project_specific_integration_names).and_call_original + subject { described_class.available_integration_names } - described_class.available_integration_names + before do + allow(described_class).to receive(:integration_names).and_return(%w(foo)) + allow(described_class).to receive(:project_specific_integration_names).and_return(['bar']) + allow(described_class).to receive(:dev_integration_names).and_return(['baz']) end - it 'does not call project_specific_integration_names with include_project_specific false' do - expect(described_class).to receive(:integration_names).and_call_original - expect(described_class).to receive(:dev_integration_names).and_call_original - expect(described_class).not_to receive(:project_specific_integration_names) + it { is_expected.to include('foo', 'bar', 'baz') } - described_class.available_integration_names(include_project_specific: false) + context 'when `include_project_specific` is false' do + subject { described_class.available_integration_names(include_project_specific: false) } + + it { is_expected.to include('foo', 'baz') } + it { is_expected.not_to include('bar') } end - it 'does not call dev_integration_names with include_dev false' do - expect(described_class).to receive(:integration_names).and_call_original - expect(described_class).not_to receive(:dev_integration_names) - expect(described_class).to receive(:project_specific_integration_names).and_call_original + context 'when `include_dev` is false' do + subject { described_class.available_integration_names(include_dev: false) } - described_class.available_integration_names(include_dev: false) + it { is_expected.to include('foo', 'bar') } + it { is_expected.not_to include('baz') } end - - it { expect(described_class.available_integration_names).to include('jenkins') } end describe '.project_specific_integration_names' do - it do + specify do expect(described_class.project_specific_integration_names) .to include(*described_class::PROJECT_SPECIFIC_INTEGRATION_NAMES) end @@ -823,4 +872,153 @@ RSpec.describe Integration do expect(subject.password_fields).to eq([]) end end + + describe 'encrypted_properties' do + let(:properties) { { foo: 1, bar: true } } + let(:db_props) { properties.stringify_keys } + let(:record) { create(:integration, :instance, properties: properties) } + + it 'contains the same data as properties' do + expect(record).to have_attributes( + properties: db_props, + encrypted_properties_tmp: db_props + ) + end + + it 'is persisted' do + encrypted_properties = described_class.id_in(record.id) + + expect(encrypted_properties).to contain_exactly have_attributes(encrypted_properties_tmp: db_props) + end + + it 'is updated when using prop_accessors' do + some_integration = Class.new(described_class) do + prop_accessor :foo + end + + record = some_integration.new + + record.foo = 'the foo' + + expect(record.encrypted_properties_tmp).to eq({ 'foo' => 'the foo' }) + end + + it 'saves correctly using insert_all' do + hash = record.to_integration_hash + hash[:project_id] = project + + expect do + described_class.insert_all([hash]) + end.to change(described_class, :count).by(1) + + expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props) + end + + it 'is part of the to_integration_hash' do + hash = record.to_integration_hash + + expect(hash).to include('encrypted_properties' => be_present, 'encrypted_properties_iv' => be_present) + expect(hash['encrypted_properties']).not_to eq(record.encrypted_properties) + expect(hash['encrypted_properties_iv']).not_to eq(record.encrypted_properties_iv) + + decrypted = described_class.decrypt(:encrypted_properties_tmp, + hash['encrypted_properties'], + { iv: hash['encrypted_properties_iv'] }) + + expect(decrypted).to eq db_props + end + + context 'when the properties are empty' do + let(:properties) { {} } + + it 'is part of the to_integration_hash' do + hash = record.to_integration_hash + + expect(hash).to include('encrypted_properties' => be_nil, 'encrypted_properties_iv' => be_nil) + end + + it 'saves correctly using insert_all' do + hash = record.to_integration_hash + hash[:project_id] = project + + expect do + described_class.insert_all([hash]) + end.to change(described_class, :count).by(1) + + expect(described_class.last).not_to eq record + expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props) + end + end + end + + describe 'field DSL' do + let(:integration_type) do + Class.new(described_class) do + field :foo + field :foo_p, storage: :properties + field :foo_dt, storage: :data_fields + + field :bar, type: 'password' + field :password + + field :with_help, + help: -> { 'help' } + + field :a_number, + type: 'number' + end + end + + before do + allow(integration).to receive(:data_fields).and_return(data_fields) + end + + let(:integration) { integration_type.new } + let(:data_fields) { Struct.new(:foo_dt).new } + + it 'checks the value of storage' do + expect do + Class.new(described_class) { field(:foo, storage: 'bar') } + end.to raise_error(ArgumentError, /Unknown field storage/) + end + + it 'provides prop_accessors' do + integration.foo = 1 + expect(integration.foo).to eq 1 + expect(integration.properties['foo']).to eq 1 + expect(integration).to be_foo_changed + + integration.foo_p = 2 + expect(integration.foo_p).to eq 2 + expect(integration.properties['foo_p']).to eq 2 + expect(integration).to be_foo_p_changed + end + + it 'provides data fields' do + integration.foo_dt = 3 + expect(integration.foo_dt).to eq 3 + expect(data_fields.foo_dt).to eq 3 + expect(integration).to be_foo_dt_changed + end + + it 'registers fields in the fields list' do + expect(integration.fields.pluck(:name)).to match_array %w[ + foo foo_p foo_dt bar password with_help a_number + ] + + expect(integration.api_field_names).to match_array %w[ + foo foo_p foo_dt with_help a_number + ] + end + + specify 'fields have expected attributes' do + expect(integration.fields).to include( + have_attributes(name: 'foo', type: 'text'), + have_attributes(name: 'bar', type: 'password'), + have_attributes(name: 'password', type: 'password'), + have_attributes(name: 'a_number', type: 'number'), + have_attributes(name: 'with_help', help: 'help') + ) + end + end end diff --git a/spec/models/integrations/base_issue_tracker_spec.rb b/spec/models/integrations/base_issue_tracker_spec.rb index 25e27e96a84..37f7d99717c 100644 --- a/spec/models/integrations/base_issue_tracker_spec.rb +++ b/spec/models/integrations/base_issue_tracker_spec.rb @@ -3,12 +3,12 @@ require 'spec_helper' RSpec.describe Integrations::BaseIssueTracker do - describe 'Validations' do - let(:project) { create :project } + let(:integration) { Integrations::Redmine.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } - describe 'only one issue tracker per project' do - let(:integration) { Integrations::Redmine.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } + let_it_be_with_refind(:project) { create :project } + describe 'Validations' do + describe 'only one issue tracker per project' do before do create(:custom_issue_tracker_integration, project: project) end @@ -31,4 +31,18 @@ RSpec.describe Integrations::BaseIssueTracker do end end end + + describe '#activate_disabled_reason' do + subject { integration.activate_disabled_reason } + + context 'when there is an existing issue tracker integration' do + let_it_be(:custom_tracker) { create(:custom_issue_tracker_integration, project: project) } + + it { is_expected.to eq(trackers: [custom_tracker]) } + end + + context 'when there is no existing issue tracker integration' do + it { is_expected.to be(nil) } + end + end end diff --git a/spec/models/integrations/field_spec.rb b/spec/models/integrations/field_spec.rb new file mode 100644 index 00000000000..0d660c4a3ab --- /dev/null +++ b/spec/models/integrations/field_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Integrations::Field do + subject(:field) { described_class.new(**attrs) } + + let(:attrs) { { name: nil } } + + describe '#name' do + before do + attrs[:name] = :foo + end + + it 'is stringified' do + expect(field.name).to eq 'foo' + expect(field[:name]).to eq 'foo' + end + + context 'when not set' do + before do + attrs.delete(:name) + end + + it 'complains' do + expect { field }.to raise_error(ArgumentError) + end + end + end + + described_class::ATTRIBUTES.each do |name| + describe "##{name}" do + it "responds to #{name}" do + expect(field).to be_respond_to(name) + end + + context 'when not set' do + before do + attrs.delete(name) + end + + let(:have_correct_default) do + case name + when :api_only + be false + when :type + eq 'text' + else + be_nil + end + end + + it 'has the correct default' do + expect(field[name]).to have_correct_default + expect(field.send(name)).to have_correct_default + end + end + + context 'when set to a static value' do + before do + attrs[name] = :known + end + + it 'is known' do + expect(field[name]).to eq(:known) + expect(field.send(name)).to eq(:known) + end + end + + context 'when set to a dynamic value' do + before do + attrs[name] = -> { Time.current } + end + + it 'is computed' do + start = Time.current + + travel_to(start + 1.minute) do + expect(field[name]).to be_after(start) + expect(field.send(name)).to be_after(start) + end + end + end + end + end + + describe '#sensitive' do + context 'when empty' do + it { is_expected.not_to be_sensitive } + end + + context 'when a password field' do + before do + attrs[:type] = 'password' + end + + it { is_expected.to be_sensitive } + end + + %w[token api_token api_key secret_key secret_sauce password passphrase].each do |name| + context "when named #{name}" do + before do + attrs[:name] = name + end + + it { is_expected.to be_sensitive } + end + end + + context "when named url" do + before do + attrs[:name] = :url + end + + it { is_expected.not_to be_sensitive } + end + end +end diff --git a/spec/models/integrations/harbor_spec.rb b/spec/models/integrations/harbor_spec.rb new file mode 100644 index 00000000000..4a6eb27d63a --- /dev/null +++ b/spec/models/integrations/harbor_spec.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::Harbor do + let(:url) { 'https://demo.goharbor.io' } + let(:project_name) { 'testproject' } + let(:username) { 'harborusername' } + let(:password) { 'harborpassword' } + let(:harbor_integration) { create(:harbor_integration) } + + describe "masked password" do + subject { build(:harbor_integration) } + + it { is_expected.not_to allow_value('hello').for(:password) } + it { is_expected.not_to allow_value('hello world').for(:password) } + it { is_expected.not_to allow_value('hello$VARIABLEworld').for(:password) } + it { is_expected.not_to allow_value('hello\rworld').for(:password) } + it { is_expected.to allow_value('helloworld').for(:password) } + end + + describe '#fields' do + it 'returns custom fields' do + expect(harbor_integration.fields.pluck(:name)).to eq(%w[url project_name username password]) + end + end + + describe '#test' do + let(:test_response) { "pong" } + + before do + allow_next_instance_of(Gitlab::Harbor::Client) do |client| + allow(client).to receive(:ping).and_return(test_response) + end + end + + it 'gets response from Gitlab::Harbor::Client#ping' do + expect(harbor_integration.test).to eq(test_response) + end + end + + describe '#help' do + it 'renders prompt information' do + expect(harbor_integration.help).not_to be_empty + end + end + + describe '.to_param' do + it 'returns the name of the integration' do + expect(described_class.to_param).to eq('harbor') + end + end + + context 'ci variables' do + it 'returns vars when harbor_integration is activated' do + ci_vars = [ + { key: 'HARBOR_URL', value: url }, + { key: 'HARBOR_PROJECT', value: project_name }, + { key: 'HARBOR_USERNAME', value: username }, + { key: 'HARBOR_PASSWORD', value: password, public: false, masked: true } + ] + + expect(harbor_integration.ci_variables).to match_array(ci_vars) + end + + it 'returns [] when harbor_integration is inactive' do + harbor_integration.update!(active: false) + expect(harbor_integration.ci_variables).to match_array([]) + end + end + + describe 'before_validation :reset_username_and_password' do + context 'when username/password was previously set' do + it 'resets username and password if url changed' do + harbor_integration.url = 'https://anotherharbor.com' + harbor_integration.valid? + + expect(harbor_integration.password).to be_nil + expect(harbor_integration.username).to be_nil + end + + it 'does not reset password if username changed' do + harbor_integration.username = 'newusername' + harbor_integration.valid? + + expect(harbor_integration.password).to eq('harborpassword') + end + + it 'does not reset username if password changed' do + harbor_integration.password = 'newpassword' + harbor_integration.valid? + + expect(harbor_integration.username).to eq('harborusername') + end + + it "does not reset password if new url is set together with password, even if it's the same password" do + harbor_integration.url = 'https://anotherharbor.com' + harbor_integration.password = 'harborpassword' + harbor_integration.valid? + + expect(harbor_integration.password).to eq('harborpassword') + expect(harbor_integration.username).to be_nil + expect(harbor_integration.url).to eq('https://anotherharbor.com') + end + + it "does not reset username if new url is set together with username, even if it's the same username" do + harbor_integration.url = 'https://anotherharbor.com' + harbor_integration.username = 'harborusername' + harbor_integration.valid? + + expect(harbor_integration.password).to be_nil + expect(harbor_integration.username).to eq('harborusername') + expect(harbor_integration.url).to eq('https://anotherharbor.com') + end + end + + it 'saves password if new url is set together with password when no password was previously set' do + harbor_integration.password = nil + harbor_integration.username = nil + + harbor_integration.url = 'https://anotherharbor.com' + harbor_integration.password = 'newpassword' + harbor_integration.username = 'newusername' + harbor_integration.save! + + expect(harbor_integration).to have_attributes( + url: 'https://anotherharbor.com', + password: 'newpassword', + username: 'newusername' + ) + end + end +end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 6ce84c28044..08656bfe543 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -109,6 +109,32 @@ RSpec.describe Integrations::Jira do end end + describe '#sections' do + let(:integration) { create(:jira_integration) } + + subject(:sections) { integration.sections.map { |s| s[:type] } } + + context 'when project_level? is true' do + before do + allow(integration).to receive(:project_level?).and_return(true) + end + + it 'includes SECTION_TYPE_JIRA_ISSUES' do + expect(sections).to include(described_class::SECTION_TYPE_JIRA_ISSUES) + end + end + + context 'when project_level? is false' do + before do + allow(integration).to receive(:project_level?).and_return(false) + end + + it 'does not include SECTION_TYPE_JIRA_ISSUES' do + expect(sections).not_to include(described_class::SECTION_TYPE_JIRA_ISSUES) + end + end + end + describe '.reference_pattern' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index 4661d9c8291..9f69f4f51f8 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Integrations::Slack do describe '#execute' do before do - stub_request(:post, "https://slack.service.url/") + stub_request(:post, slack_integration.webhook) end let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all') } diff --git a/spec/models/issue_link_spec.rb b/spec/models/issue_link_spec.rb index 433b51b8a70..9f77fcef5da 100644 --- a/spec/models/issue_link_spec.rb +++ b/spec/models/issue_link_spec.rb @@ -3,57 +3,42 @@ require 'spec_helper' RSpec.describe IssueLink do - describe 'Associations' do - it { is_expected.to belong_to(:source).class_name('Issue') } - it { is_expected.to belong_to(:target).class_name('Issue') } + it_behaves_like 'issuable link' do + let_it_be_with_reload(:issuable_link) { create(:issue_link) } + let_it_be(:issuable) { create(:issue) } + let(:issuable_class) { 'Issue' } + let(:issuable_link_factory) { :issue_link } end - describe 'link_type' do - it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1) } - - it 'provides the "related" as default link_type' do - expect(create(:issue_link).link_type).to eq 'relates_to' - end + describe '.issuable_type' do + it { expect(described_class.issuable_type).to eq(:issue) } end - describe 'Validation' do - subject { create :issue_link } + describe 'Scopes' do + let_it_be(:issue1) { create(:issue) } + let_it_be(:issue2) { create(:issue) } - it { is_expected.to validate_presence_of(:source) } - it { is_expected.to validate_presence_of(:target) } - it do - is_expected.to validate_uniqueness_of(:source) - .scoped_to(:target_id) - .with_message(/already related/) - end + describe '.for_source_issue' do + it 'includes linked issues for source issue' do + source_issue = create(:issue) + issue_link_1 = create(:issue_link, source: source_issue, target: issue1) + issue_link_2 = create(:issue_link, source: source_issue, target: issue2) - it 'is not valid if an opposite link already exists' do - issue_link = build(:issue_link, source: subject.target, target: subject.source) + result = described_class.for_source_issue(source_issue) - expect(issue_link).to be_invalid - expect(issue_link.errors[:source]).to include('is already related to this issue') + expect(result).to contain_exactly(issue_link_1, issue_link_2) + end end - context 'when it relates to itself' do - let(:issue) { create :issue } - - context 'cannot be validated' do - it 'does not invalidate object with self relation error' do - issue_link = build :issue_link, source: issue, target: nil - - issue_link.valid? - - expect(issue_link.errors[:source]).to be_empty - end - end + describe '.for_target_issue' do + it 'includes linked issues for target issue' do + target_issue = create(:issue) + issue_link_1 = create(:issue_link, source: issue1, target: target_issue) + issue_link_2 = create(:issue_link, source: issue2, target: target_issue) - context 'can be invalidated' do - it 'invalidates object' do - issue_link = build :issue_link, source: issue, target: issue + result = described_class.for_target_issue(target_issue) - expect(issue_link).to be_invalid - expect(issue_link.errors[:source]).to include('cannot be related to itself') - end + expect(result).to contain_exactly(issue_link_1, issue_link_2) end end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 5af42cc67ea..29305ba435c 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1167,7 +1167,6 @@ RSpec.describe Issue do end describe '#check_for_spam?' do - using RSpec::Parameterized::TableSyntax let_it_be(:support_bot) { ::User.support_bot } where(:support_bot?, :visibility_level, :confidential, :new_attributes, :check_for_spam?) do diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 14acaf11ca4..ff7ac0ebd2a 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -67,24 +67,21 @@ RSpec.describe Label do label = described_class.new(color: ' #abcdef ') label.valid? - expect(label.color).to eq('#abcdef') + expect(label.color).to be_color('#abcdef') end it 'uses default color if color is missing' do label = described_class.new(color: nil) - expect(label.color).to be(Label::DEFAULT_COLOR) + expect(label.color).to be_color(Label::DEFAULT_COLOR) end end describe '#text_color' do it 'uses default color if color is missing' do - expect(LabelsHelper).to receive(:text_color_for_bg).with(Label::DEFAULT_COLOR) - .and_return(spy) - label = described_class.new(color: nil) - label.text_color + expect(label.text_color).to eq(Label::DEFAULT_COLOR.contrast) end end @@ -107,6 +104,12 @@ RSpec.describe Label do label = described_class.new(description: '<b>foo & bar?</b>') expect(label.description).to eq('foo & bar?') end + + it 'accepts an empty string' do + label = described_class.new(title: 'foo', description: 'bar') + label.update!(description: '') + expect(label.description).to eq('') + end end describe 'priorization' do diff --git a/spec/models/merge_request_assignee_spec.rb b/spec/models/merge_request_assignee_spec.rb index 58b802de8e0..1591c517049 100644 --- a/spec/models/merge_request_assignee_spec.rb +++ b/spec/models/merge_request_assignee_spec.rb @@ -51,4 +51,24 @@ RSpec.describe MergeRequestAssignee do it { is_expected.to have_attributes(state: 'reviewed') } end + + describe '#attention_requested_by' do + let(:current_user) { create(:user) } + + before do + subject.update!(updated_state_by: current_user) + end + + context 'attention requested' do + it { expect(subject.attention_requested_by).to eq(current_user) } + end + + context 'attention requested' do + before do + subject.update!(state: :reviewed) + end + + it { expect(subject.attention_requested_by).to eq(nil) } + end + end end diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb index d99fd4afb0f..dd00c4d8627 100644 --- a/spec/models/merge_request_reviewer_spec.rb +++ b/spec/models/merge_request_reviewer_spec.rb @@ -25,4 +25,24 @@ RSpec.describe MergeRequestReviewer do it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') } it { is_expected.to belong_to(:reviewer).class_name('User').inverse_of(:merge_request_reviewers) } end + + describe '#attention_requested_by' do + let(:current_user) { create(:user) } + + before do + subject.update!(updated_state_by: current_user) + end + + context 'attention requested' do + it { expect(subject.attention_requested_by).to eq(current_user) } + end + + context 'attention requested' do + before do + subject.update!(state: :reviewed) + end + + it { expect(subject.attention_requested_by).to eq(nil) } + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index f2f2023a992..0d15851e583 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3225,52 +3225,44 @@ RSpec.describe MergeRequest, factory_default: :keep do end context 'when failed' do - shared_examples 'failed skip_ci_check' do - context 'when #mergeable_ci_state? is false' do - before do - allow(subject).to receive(:mergeable_ci_state?) { false } - end - - it 'returns false' do - expect(subject.mergeable_state?).to be_falsey - end - - it 'returns true when skipping ci check' do - expect(subject.mergeable_state?(skip_ci_check: true)).to be(true) - end + context 'when #mergeable_ci_state? is false' do + before do + allow(subject).to receive(:mergeable_ci_state?) { false } end - context 'when #mergeable_discussions_state? is false' do - before do - allow(subject).to receive(:mergeable_discussions_state?) { false } - end - - it 'returns false' do - expect(subject.mergeable_state?).to be_falsey - end - - it 'returns true when skipping discussions check' do - expect(subject.mergeable_state?(skip_discussions_check: true)).to be(true) - end + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey end - end - context 'when improved_mergeability_checks is on' do - it_behaves_like 'failed skip_ci_check' + it 'returns true when skipping ci check' do + expect(subject.mergeable_state?(skip_ci_check: true)).to be(true) + end end - context 'when improved_mergeability_checks is off' do + context 'when #mergeable_discussions_state? is false' do before do - stub_feature_flags(improved_mergeability_checks: false) + allow(subject).to receive(:mergeable_discussions_state?) { false } end - it_behaves_like 'failed skip_ci_check' + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + + it 'returns true when skipping discussions check' do + expect(subject.mergeable_state?(skip_discussions_check: true)).to be(true) + end end end end describe '#mergeable_state?' do - context 'when merge state caching is on' do + it_behaves_like 'for mergeable_state' + + context 'when improved_mergeability_checks is off' do + before do + stub_feature_flags(improved_mergeability_checks: false) + end + it_behaves_like 'for mergeable_state' end @@ -4249,6 +4241,29 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#eager_fetch_ref!' do + let(:project) { create(:project, :repository) } + + # We use build instead of create to test that an IID is allocated + subject { build(:merge_request, source_project: project) } + + it 'fetches the ref correctly' do + expect(subject.iid).to be_nil + + expect { subject.eager_fetch_ref! }.to change { subject.iid.to_i }.by(1) + + expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy + end + + it 'only fetches the ref once after saved' do + expect(subject.target_project.repository).to receive(:fetch_source_branch!).once.and_call_original + + subject.save! + + expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy + end + end + describe 'removing a merge request' do it 'refreshes the number of open merge requests of the target project' do project = subject.target_project @@ -5086,4 +5101,34 @@ RSpec.describe MergeRequest, factory_default: :keep do let!(:model) { create(:merge_request, head_pipeline: parent) } end end + + describe '#merge_request_reviewers_with' do + let_it_be(:reviewer1) { create(:user) } + let_it_be(:reviewer2) { create(:user) } + + before do + subject.update!(reviewers: [reviewer1, reviewer2]) + end + + it 'returns reviewers' do + reviewers = subject.merge_request_reviewers_with([reviewer1.id]) + + expect(reviewers).to match_array([subject.merge_request_reviewers[0]]) + end + end + + describe '#merge_request_assignees_with' do + let_it_be(:assignee1) { create(:user) } + let_it_be(:assignee2) { create(:user) } + + before do + subject.update!(assignees: [assignee1, assignee2]) + end + + it 'returns assignees' do + assignees = subject.merge_request_assignees_with([assignee1.id]) + + expect(assignees).to match_array([subject.merge_request_assignees[0]]) + end + end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index bc592acc80f..06044cf53cc 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -65,6 +65,17 @@ RSpec.describe Milestone do allow(subject).to receive(:set_iid).and_return(false) end + describe 'title' do + it { is_expected.to validate_presence_of(:title) } + + it 'is invalid if title would be empty after sanitation', :aggregate_failures do + milestone = build(:milestone, project: project, title: '<img src=x onerror=prompt(1)>') + + expect(milestone).not_to be_valid + expect(milestone.errors[:title]).to include("can't be blank") + end + end + describe 'milestone_releases' do let(:milestone) { build(:milestone, project: project) } diff --git a/spec/models/namespace/root_storage_statistics_spec.rb b/spec/models/namespace/root_storage_statistics_spec.rb index 11852828eab..c399a0084fb 100644 --- a/spec/models/namespace/root_storage_statistics_spec.rb +++ b/spec/models/namespace/root_storage_statistics_spec.rb @@ -178,7 +178,7 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do snippets = create_list(:personal_snippet, 3, :repository, author: user) snippets.each { |s| s.statistics.refresh! } - total_personal_snippets_size = snippets.map { |s| s.statistics.repository_size }.sum + total_personal_snippets_size = snippets.sum { |s| s.statistics.repository_size } root_storage_statistics.recalculate! diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 1728d4fc3f3..ebd153f6f10 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -436,17 +436,7 @@ RSpec.describe Namespace do it { expect(namespace.traversal_ids).to eq [namespace.id] } end - context 'with before_commit callback' do - it_behaves_like 'default traversal_ids' - end - - context 'with after_create callback' do - before do - stub_feature_flags(sync_traversal_ids_before_commit: false) - end - - it_behaves_like 'default traversal_ids' - end + it_behaves_like 'default traversal_ids' end describe "after_commit :expire_child_caches" do diff --git a/spec/models/packages/pypi/metadatum_spec.rb b/spec/models/packages/pypi/metadatum_spec.rb index 2c9893ef8f3..6c83c4ed143 100644 --- a/spec/models/packages/pypi/metadatum_spec.rb +++ b/spec/models/packages/pypi/metadatum_spec.rb @@ -8,6 +8,9 @@ RSpec.describe Packages::Pypi::Metadatum, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:package) } + it { is_expected.to allow_value('').for(:required_python) } + it { is_expected.not_to allow_value(nil).for(:required_python) } + it { is_expected.not_to allow_value('a' * 256).for(:required_python) } describe '#pypi_package_type' do it 'will not allow a package with a different package_type' do diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 88206fbf48c..125ac7fb102 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -32,6 +32,17 @@ RSpec.describe PersonalAccessToken do it { is_expected.to contain_exactly(project_access_token) } end + describe '.owner_is_human' do + let_it_be(:user) { create(:user, :project_bot) } + let_it_be(:project_member) { create(:project_member, user: user) } + let_it_be(:personal_access_token) { create(:personal_access_token) } + let_it_be(:project_access_token) { create(:personal_access_token, user: user) } + + subject { described_class.owner_is_human } + + it { is_expected.to contain_exactly(personal_access_token) } + end + describe '.for_user' do it 'returns personal access tokens of specified user only' do user_1 = create(:user) diff --git a/spec/models/preloaders/environments/deployment_preloader_spec.rb b/spec/models/preloaders/environments/deployment_preloader_spec.rb index c1812d45628..3f2f28a069e 100644 --- a/spec/models/preloaders/environments/deployment_preloader_spec.rb +++ b/spec/models/preloaders/environments/deployment_preloader_spec.rb @@ -62,4 +62,22 @@ RSpec.describe Preloaders::Environments::DeploymentPreloader do expect(default_preload_query).to be(false) end + + it 'sets environment on the associated deployment', :aggregate_failures do + preload_association(:last_deployment) + + expect do + project.environments.each { |environment| environment.last_deployment.environment } + end.not_to exceed_query_limit(0) + + project.environments.each do |environment| + expect(environment.last_deployment.environment).to eq(environment) + end + end + + it 'does not attempt to set environment on a nil deployment' do + create(:environment, project: project, state: :available) + + expect { preload_association(:last_deployment) }.not_to raise_error + end end diff --git a/spec/models/project_authorization_spec.rb b/spec/models/project_authorization_spec.rb index 37da30fb54c..14220007966 100644 --- a/spec/models/project_authorization_spec.rb +++ b/spec/models/project_authorization_spec.rb @@ -3,6 +3,56 @@ require 'spec_helper' RSpec.describe ProjectAuthorization do + describe 'unique user, project authorizations' do + let_it_be(:user) { create(:user) } + let_it_be(:project_1) { create(:project) } + + let!(:project_auth) do + create( + :project_authorization, + user: user, + project: project_1, + access_level: Gitlab::Access::DEVELOPER + ) + end + + context 'with duplicate user and project authorization' do + subject { project_auth.dup } + + it { is_expected.to be_invalid } + + context 'after validation' do + before do + subject.valid? + end + + it 'contains duplicate error' do + expect(subject.errors[:user]).to include('has already been taken') + end + end + end + + context 'with multiple access levels for the same user and project' do + subject do + project_auth.dup.tap do |auth| + auth.access_level = Gitlab::Access::MAINTAINER + end + end + + it { is_expected.to be_invalid } + + context 'after validation' do + before do + subject.valid? + end + + it 'contains duplicate error' do + expect(subject.errors[:user]).to include('has already been taken') + end + end + end + end + describe 'relations' do it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:project) } diff --git a/spec/models/project_pages_metadatum_spec.rb b/spec/models/project_pages_metadatum_spec.rb index af2f9b94871..31a533e0363 100644 --- a/spec/models/project_pages_metadatum_spec.rb +++ b/spec/models/project_pages_metadatum_spec.rb @@ -18,15 +18,4 @@ RSpec.describe ProjectPagesMetadatum do expect(described_class.only_on_legacy_storage).to eq([legacy_storage_project.pages_metadatum]) end end - - it_behaves_like 'cleanup by a loose foreign key' do - let!(:model) do - artifacts_archive = create(:ci_job_artifact, :legacy_archive) - metadatum = artifacts_archive.project.pages_metadatum - metadatum.artifacts_archive = artifacts_archive - metadatum - end - - let!(:parent) { model.artifacts_archive } - end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1d9b38c7aa4..fc7ac35ed41 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -64,6 +64,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_one(:bamboo_integration) } it { is_expected.to have_one(:teamcity_integration) } it { is_expected.to have_one(:jira_integration) } + it { is_expected.to have_one(:harbor_integration) } it { is_expected.to have_one(:redmine_integration) } it { is_expected.to have_one(:youtrack_integration) } it { is_expected.to have_one(:custom_issue_tracker_integration) } @@ -134,7 +135,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:packages).class_name('Packages::Package') } it { is_expected.to have_many(:package_files).class_name('Packages::PackageFile') } it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::ProjectDistribution').dependent(:destroy) } - it { is_expected.to have_many(:pipeline_artifacts) } + it { is_expected.to have_many(:pipeline_artifacts).dependent(:restrict_with_error) } it { is_expected.to have_many(:terraform_states).class_name('Terraform::State').inverse_of(:project) } it { is_expected.to have_many(:timelogs) } it { is_expected.to have_many(:error_tracking_errors).class_name('ErrorTracking::Error') } @@ -142,6 +143,9 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:pending_builds).class_name('Ci::PendingBuild') } it { is_expected.to have_many(:ci_feature_usages).class_name('Projects::CiFeatureUsage') } it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } + it { is_expected.to have_many(:job_artifacts).dependent(:restrict_with_error) } + it { is_expected.to have_many(:build_trace_chunks).through(:builds).dependent(:restrict_with_error) } + it { is_expected.to have_many(:secure_files).class_name('Ci::SecureFile').dependent(:restrict_with_error) } # GitLab Pages it { is_expected.to have_many(:pages_domains) } @@ -202,6 +206,35 @@ RSpec.describe Project, factory_default: :keep do end end + context 'when project has object storage attached to it' do + let_it_be(:project) { create(:project) } + + before do + create(:ci_job_artifact, project: project) + end + + context 'when associated object storage object is not deleted before the project' do + it 'adds an error to project', :aggregate_failures do + expect { project.destroy! }.to raise_error(ActiveRecord::RecordNotDestroyed) + + expect(project.errors).not_to be_empty + expect(project.errors.first.message).to eq("Cannot delete record because dependent job artifacts exist") + end + end + + context 'when associated object storage object is deleted before the project' do + before do + project.job_artifacts.first.destroy! + end + + it 'deletes the project' do + project.destroy! + + expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + context 'when creating a new project' do let_it_be(:project) { create(:project) } @@ -246,33 +279,9 @@ RSpec.describe Project, factory_default: :keep do expect(project.project_namespace).to be_in_sync_with_project(project) expect(project.reload.project_namespace.traversal_ids).to eq([project.namespace.traversal_ids, project.project_namespace.id].flatten.compact) end - - context 'with FF disabled' do - before do - stub_feature_flags(create_project_namespace_on_project_create: false) - end - - it 'does not create a project namespace' do - project = build(:project, path: 'hopefully-valid-path2') - project.save! - - expect(project).to be_persisted - expect(project.project_namespace).to be_nil - end - end - end - - context 'sync-ing traversal_ids in before_commit callback' do - it_behaves_like 'creates project namespace' end - context 'sync-ing traversal_ids in after_create callback' do - before do - stub_feature_flags(sync_traversal_ids_before_commit: false) - end - - it_behaves_like 'creates project namespace' - end + it_behaves_like 'creates project namespace' end end @@ -316,35 +325,6 @@ RSpec.describe Project, factory_default: :keep do end end end - - context 'with create_project_namespace_on_project_create FF enabled' do - it_behaves_like 'project update' - - it 'keeps project namespace in sync with project' do - project = create(:project) - project.update!(path: 'hopefully-valid-path1') - - expect(project).to be_persisted - expect(project.project_namespace).to be_persisted - expect(project.project_namespace).to be_in_sync_with_project(project) - end - end - - context 'with create_project_namespace_on_project_create FF disabled' do - before do - stub_feature_flags(create_project_namespace_on_project_create: false) - end - - it_behaves_like 'project update' - - it 'does not create a project namespace when project is updated' do - project = create(:project) - project.update!(path: 'hopefully-valid-path1') - - expect(project).to be_persisted - expect(project.project_namespace).to be_nil - end - end end context 'updating cd_cd_settings' do @@ -627,10 +607,30 @@ RSpec.describe Project, factory_default: :keep do expect(project).to be_valid end - it 'allows a path ending in a period' do - project = build(:project, path: 'foo.') + context 'path is unchanged' do + let_it_be(:invalid_path_project) do + project = create(:project, :repository, :public) + project.update_attribute(:path, 'foo.') + project + end - expect(project).to be_valid + it 'does not raise validation error for path for existing project' do + expect { invalid_path_project.update!(name: 'Foo') }.not_to raise_error + end + end + + %w[. - _].each do |special_character| + it "rejects a path ending in '#{special_character}'" do + project = build(:project, path: "foo#{special_character}") + + expect(project).not_to be_valid + end + + it "rejects a path starting with '#{special_character}'" do + project = build(:project, path: "#{special_character}foo") + + expect(project).not_to be_valid + end end end end @@ -782,8 +782,8 @@ RSpec.describe Project, factory_default: :keep do end it 'does not set an random token if one provided' do - project = FactoryBot.create(:project, runners_token: "#{Project::RUNNERS_TOKEN_PREFIX}my-token") - expect(project.runners_token).to eq("#{Project::RUNNERS_TOKEN_PREFIX}my-token") + project = FactoryBot.create(:project, runners_token: "#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}my-token") + expect(project.runners_token).to eq("#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}my-token") end end @@ -1470,7 +1470,7 @@ RSpec.describe Project, factory_default: :keep do context 'when there is an active external issue tracker integration' do let!(:integration) do - create(:integration, project: project, type: 'JiraService', category: 'issue_tracker', active: true) + create(:jira_integration, project: project, category: 'issue_tracker') end specify { is_expected.to eq(true) } @@ -1489,7 +1489,7 @@ RSpec.describe Project, factory_default: :keep do context 'when there are two active external issue tracker integrations' do let_it_be(:second_integration) do - create(:integration, project: project, type: 'CustomIssueTracker', category: 'issue_tracker', active: true) + create(:custom_issue_tracker_integration, project: project, category: 'issue_tracker') end it 'does not become false when external issue tracker integration is destroyed' do @@ -6559,7 +6559,6 @@ RSpec.describe Project, factory_default: :keep do describe '#mark_pages_as_deployed' do let(:project) { create(:project) } - let(:artifacts_archive) { create(:ci_job_artifact, project: project) } it "works when artifacts_archive is missing" do project.mark_pages_as_deployed @@ -6571,7 +6570,7 @@ RSpec.describe Project, factory_default: :keep do project.pages_metadatum.destroy! project.reload - project.mark_pages_as_deployed(artifacts_archive: artifacts_archive) + project.mark_pages_as_deployed expect(project.pages_metadatum.reload.deployed).to eq(true) end @@ -6581,15 +6580,13 @@ RSpec.describe Project, factory_default: :keep do pages_metadatum.update!(deployed: false) expect do - project.mark_pages_as_deployed(artifacts_archive: artifacts_archive) + project.mark_pages_as_deployed end.to change { pages_metadatum.reload.deployed }.from(false).to(true) - .and change { pages_metadatum.reload.artifacts_archive }.from(nil).to(artifacts_archive) end end describe '#mark_pages_as_not_deployed' do let(:project) { create(:project) } - let(:artifacts_archive) { create(:ci_job_artifact, project: project) } it "creates new record and sets deployed to false if none exists yet" do project.pages_metadatum.destroy! @@ -6602,12 +6599,11 @@ RSpec.describe Project, factory_default: :keep do it "updates the existing record and sets deployed to false and clears artifacts_archive" do pages_metadatum = project.pages_metadatum - pages_metadatum.update!(deployed: true, artifacts_archive: artifacts_archive) + pages_metadatum.update!(deployed: true) expect do project.mark_pages_as_not_deployed end.to change { pages_metadatum.reload.deployed }.from(true).to(false) - .and change { pages_metadatum.reload.artifacts_archive }.from(artifacts_archive).to(nil) end end @@ -6697,6 +6693,24 @@ RSpec.describe Project, factory_default: :keep do end describe '#access_request_approvers_to_be_notified' do + context 'for a personal project' do + let_it_be(:project) { create(:project) } + let_it_be(:maintainer) { create(:user) } + + let(:owner_membership) { project.members.owners.find_by(user_id: project.namespace.owner_id) } + + it 'includes only the owner of the personal project' do + expect(project.access_request_approvers_to_be_notified.to_a).to eq([owner_membership]) + end + + it 'includes the maintainers of the personal project, if any' do + project.add_maintainer(maintainer) + maintainer_membership = project.members.maintainers.find_by(user_id: maintainer.id) + + expect(project.access_request_approvers_to_be_notified.to_a).to match_array([owner_membership, maintainer_membership]) + end + end + let_it_be(:project) { create(:project, group: create(:group, :public)) } it 'returns a maximum of ten maintainers of the project in recent_sign_in descending order' do @@ -7504,6 +7518,14 @@ RSpec.describe Project, factory_default: :keep do expect(project.save).to be_falsy expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) end + + it 'does not add new topic if name is not unique (case insensitive)' do + project.topic_list = 'topic1, TOPIC2, topic3' + + project.save! + + expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) + end end context 'public topics counter' do @@ -7956,53 +7978,41 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#context_commits_enabled?' do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, namespace: group) } - - subject(:result) { project.context_commits_enabled? } - - context 'when context_commits feature flag is enabled' do - before do - stub_feature_flags(context_commits: true) - end - - it { is_expected.to be_truthy } - end - - context 'when context_commits feature flag is disabled' do - before do - stub_feature_flags(context_commits: false) - end + describe '.not_hidden' do + it 'lists projects that are not hidden' do + project = create(:project) + hidden_project = create(:project, :hidden) - it { is_expected.to be_falsey } + expect(described_class.not_hidden).to contain_exactly(project) + expect(described_class.not_hidden).not_to include(hidden_project) end + end - context 'when context_commits feature flag is enabled on project group' do - before do - stub_feature_flags(context_commits: group) - end + describe '#pending_delete_or_hidden?' do + let_it_be(:project) { create(:project, name: 'test-project') } - it { is_expected.to be_truthy } + where(:pending_delete, :hidden, :expected_result) do + true | false | true + true | true | true + false | true | true + false | false | false end - context 'when context_commits feature flag is enabled on another group' do - let(:another_group) { create(:group) } + with_them do + it 'returns true if project is pending delete or hidden' do + project.pending_delete = pending_delete + project.hidden = hidden + project.save! - before do - stub_feature_flags(context_commits: another_group) + expect(project.pending_delete_or_hidden?).to eq(expected_result) end - - it { is_expected.to be_falsey } end end - describe '#runners_token' do - let_it_be(:project) { create(:project) } - - subject { project } + describe 'serialization' do + let(:object) { build(:project) } - it_behaves_like 'it has a prefixable runners_token' + it_behaves_like 'blocks unsafe serialization' end private diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index bfdebbc33df..5b11f9d828a 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -225,7 +225,7 @@ RSpec.describe ProjectTeam do let_it_be(:maintainer) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:guest) { create(:user) } - let_it_be(:project) { create(:project, namespace: maintainer.namespace) } + let_it_be(:project) { create(:project, group: create(:group)) } let_it_be(:access_levels) { [Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER] } subject(:members_with_access_levels) { project.team.members_with_access_levels(access_levels) } diff --git a/spec/models/projects/build_artifacts_size_refresh_spec.rb b/spec/models/projects/build_artifacts_size_refresh_spec.rb new file mode 100644 index 00000000000..22c27c986f8 --- /dev/null +++ b/spec/models/projects/build_artifacts_size_refresh_spec.rb @@ -0,0 +1,227 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + it_behaves_like 'having unique enum values' + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + end + + describe 'scopes' do + let_it_be(:refresh_1) { create(:project_build_artifacts_size_refresh, :running, updated_at: 4.days.ago) } + let_it_be(:refresh_2) { create(:project_build_artifacts_size_refresh, :running, updated_at: 2.days.ago) } + let_it_be(:refresh_3) { create(:project_build_artifacts_size_refresh, :pending) } + let_it_be(:refresh_4) { create(:project_build_artifacts_size_refresh, :created) } + + describe 'stale' do + it 'returns records in running state and has not been updated for more than 3 days' do + expect(described_class.stale).to eq([refresh_1]) + end + end + + describe 'remaining' do + it 'returns stale, created, and pending records' do + expect(described_class.remaining).to match_array([refresh_1, refresh_3, refresh_4]) + end + end + end + + describe 'state machine', :clean_gitlab_redis_shared_state do + around do |example| + freeze_time { example.run } + end + + let(:now) { Time.zone.now } + + describe 'initial state' do + let(:refresh) { create(:project_build_artifacts_size_refresh) } + + it 'defaults to created' do + expect(refresh).to be_created + end + end + + describe '#process!' do + context 'when refresh state is created' do + let!(:refresh) do + create( + :project_build_artifacts_size_refresh, + :created, + updated_at: 2.days.ago, + refresh_started_at: nil, + last_job_artifact_id: nil + ) + end + + before do + stats = create(:project_statistics, project: refresh.project, build_artifacts_size: 120) + stats.increment_counter(:build_artifacts_size, 30) + end + + it 'transitions the state to running' do + expect { refresh.process! }.to change { refresh.reload.state }.to(described_class::STATES[:running]) + end + + it 'sets the refresh_started_at' do + expect { refresh.process! }.to change { refresh.reload.refresh_started_at.to_i }.to(now.to_i) + end + + it 'bumps the updated_at' do + expect { refresh.process! }.to change { refresh.reload.updated_at.to_i }.to(now.to_i) + end + + it 'resets the build artifacts size stats' do + expect { refresh.process! }.to change { refresh.project.statistics.reload.build_artifacts_size }.to(0) + end + + it 'resets the counter attribute to zero' do + expect { refresh.process! }.to change { refresh.project.statistics.get_counter_value(:build_artifacts_size) }.to(0) + end + end + + context 'when refresh state is pending' do + let!(:refresh) do + create( + :project_build_artifacts_size_refresh, + :pending, + updated_at: 2.days.ago + ) + end + + before do + create(:project_statistics, project: refresh.project) + end + + it 'transitions the state to running' do + expect { refresh.process! }.to change { refresh.reload.state }.to(described_class::STATES[:running]) + end + + it 'bumps the updated_at' do + expect { refresh.process! }.to change { refresh.reload.updated_at.to_i }.to(now.to_i) + end + end + + context 'when refresh state is running' do + let!(:refresh) do + create( + :project_build_artifacts_size_refresh, + :running, + updated_at: 2.days.ago + ) + end + + before do + create(:project_statistics, project: refresh.project) + end + + it 'keeps the state at running' do + expect { refresh.process! }.not_to change { refresh.reload.state } + end + + it 'bumps the updated_at' do + # If this was a stale job, we want to bump the updated at now so that + # it won't be picked up by another worker while we're recalculating + expect { refresh.process! }.to change { refresh.reload.updated_at.to_i }.to(now.to_i) + end + end + end + + describe '#requeue!' do + let!(:refresh) do + create( + :project_build_artifacts_size_refresh, + :running, + updated_at: 2.days.ago, + last_job_artifact_id: 111 + ) + end + + let(:last_job_artifact_id) { 123 } + + it 'transitions refresh state from running to pending' do + expect { refresh.requeue!(last_job_artifact_id) }.to change { refresh.reload.state }.to(described_class::STATES[:pending]) + end + + it 'bumps updated_at' do + expect { refresh.requeue!(last_job_artifact_id) }.to change { refresh.reload.updated_at.to_i }.to(now.to_i) + end + + it 'updates last_job_artifact_id' do + expect { refresh.requeue!(last_job_artifact_id) }.to change { refresh.reload.last_job_artifact_id.to_i }.to(last_job_artifact_id) + end + end + end + + describe '.process_next_refresh!' do + let!(:refresh_running) { create(:project_build_artifacts_size_refresh, :running) } + let!(:refresh_created) { create(:project_build_artifacts_size_refresh, :created) } + let!(:refresh_stale) { create(:project_build_artifacts_size_refresh, :stale) } + let!(:refresh_pending) { create(:project_build_artifacts_size_refresh, :pending) } + + subject(:processed_refresh) { described_class.process_next_refresh! } + + it 'picks the first record from the remaining work' do + expect(processed_refresh).to eq(refresh_created) + expect(processed_refresh.reload).to be_running + end + end + + describe '.enqueue_refresh' do + let_it_be(:project_1) { create(:project) } + let_it_be(:project_2) { create(:project) } + + let(:projects) { [project_1, project_1, project_2] } + + it 'creates refresh records for each given project, skipping duplicates' do + expect { described_class.enqueue_refresh(projects) } + .to change { described_class.count }.from(0).to(2) + + expect(described_class.first).to have_attributes( + project_id: project_1.id, + last_job_artifact_id: nil, + refresh_started_at: nil, + state: described_class::STATES[:created] + ) + + expect(described_class.last).to have_attributes( + project_id: project_2.id, + last_job_artifact_id: nil, + refresh_started_at: nil, + state: described_class::STATES[:created] + ) + end + end + + describe '#next_batch' do + let!(:project) { create(:project) } + let!(:artifact_1) { create(:ci_job_artifact, project: project, created_at: 14.days.ago) } + let!(:artifact_2) { create(:ci_job_artifact, project: project, created_at: 13.days.ago) } + let!(:artifact_3) { create(:ci_job_artifact, project: project, created_at: 12.days.ago) } + + # This should not be included in the recalculation as it is created later than the refresh start time + let!(:future_artifact) { create(:ci_job_artifact, project: project, size: 8, created_at: refresh.refresh_started_at + 1.second) } + + let!(:refresh) do + create( + :project_build_artifacts_size_refresh, + :pending, + project: project, + updated_at: 2.days.ago, + refresh_started_at: 10.days.ago, + last_job_artifact_id: artifact_1.id + ) + end + + subject(:batch) { refresh.next_batch(limit: 3) } + + it 'returns the job artifact records that were created not later than the refresh_started_at and IDs greater than the last_job_artifact_id' do + expect(batch).to eq([artifact_2, artifact_3]) + end + end +end diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb index 397c65f4d5c..aa3230da1e6 100644 --- a/spec/models/projects/topic_spec.rb +++ b/spec/models/projects/topic_spec.rb @@ -22,22 +22,22 @@ RSpec.describe Projects::Topic do describe 'validations' do it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_uniqueness_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).case_insensitive } it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_length_of(:description).is_at_most(1024) } end describe 'scopes' do - describe 'order_by_total_projects_count' do + describe 'order_by_non_private_projects_count' do let!(:topic1) { create(:topic, name: 'topicB') } let!(:topic2) { create(:topic, name: 'topicC') } let!(:topic3) { create(:topic, name: 'topicA') } - let!(:project1) { create(:project, topic_list: 'topicC, topicA, topicB') } - let!(:project2) { create(:project, topic_list: 'topicC, topicA') } - let!(:project3) { create(:project, topic_list: 'topicC') } + let!(:project1) { create(:project, :public, topic_list: 'topicC, topicA, topicB') } + let!(:project2) { create(:project, :public, topic_list: 'topicC, topicA') } + let!(:project3) { create(:project, :public, topic_list: 'topicC') } - it 'sorts topics by total_projects_count' do - topics = described_class.order_by_total_projects_count + it 'sorts topics by non_private_projects_count' do + topics = described_class.order_by_non_private_projects_count expect(topics.map(&:name)).to eq(%w[topicC topicA topicB topic]) end diff --git a/spec/models/projects/triggered_hooks_spec.rb b/spec/models/projects/triggered_hooks_spec.rb new file mode 100644 index 00000000000..3c885bdac8e --- /dev/null +++ b/spec/models/projects/triggered_hooks_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::TriggeredHooks do + let_it_be(:project) { create(:project) } + + let_it_be(:universal_push_hook) { create(:project_hook, project: project, push_events: true) } + let_it_be(:selective_push_hook) { create(:project_hook, :with_push_branch_filter, project: project, push_events: true) } + let_it_be(:issues_hook) { create(:project_hook, project: project, issues_events: true, push_events: false) } + + let(:wh_service) { instance_double(::WebHookService, async_execute: true) } + + def run_hooks(scope, data) + hooks = described_class.new(scope, data) + hooks.add_hooks(ProjectHook.all) + hooks.execute + end + + it 'executes hooks by scope' do + data = { some: 'data', as: 'json' } + + expect_hook_execution(issues_hook, data, 'issue_hooks') + + run_hooks(:issue_hooks, data) + end + + it 'applies branch filters, when they match' do + data = { some: 'data', as: 'json', ref: "refs/heads/#{generate(:branch)}" } + + expect_hook_execution(universal_push_hook, data, 'push_hooks') + expect_hook_execution(selective_push_hook, data, 'push_hooks') + + run_hooks(:push_hooks, data) + end + + it 'applies branch filters, when they do not match' do + data = { some: 'data', as: 'json', ref: "refs/heads/master}" } + + expect_hook_execution(universal_push_hook, data, 'push_hooks') + + run_hooks(:push_hooks, data) + end + + def expect_hook_execution(hook, data, scope) + expect(WebHookService).to receive(:new).with(hook, data, scope).and_return(wh_service) + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index e592a4964f5..215f83adf5d 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3062,6 +3062,14 @@ RSpec.describe Repository do repository.create_if_not_exists end + it 'creates a repository with a default branch name' do + default_branch_name = 'branch-a' + repository.create_if_not_exists(default_branch_name) + repository.create_file(user, 'file', 'content', message: 'initial commit', branch_name: default_branch_name) + + expect(repository.root_ref).to eq(default_branch_name) + end + context 'it does nothing if the repository already existed' do let(:project) { create(:project, :repository) } diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 92e4bc7d1a9..70afafce132 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -667,6 +667,16 @@ RSpec.describe Snippet do expect(snippet.repository.exists?).to be_truthy end + it 'sets the default branch' do + expect(snippet).to receive(:default_branch).and_return('default-branch-1') + expect(subject).to be_truthy + + snippet.repository.create_file(snippet.author, 'file', 'content', message: 'initial commit', branch_name: 'default-branch-1') + + expect(snippet.repository.exists?).to be_truthy + expect(snippet.repository.root_ref).to eq('default-branch-1') + end + it 'tracks snippet repository' do expect do subject @@ -677,6 +687,7 @@ RSpec.describe Snippet do expect(snippet).to receive(:repository_storage).and_return('picked') expect(snippet).to receive(:repository_exists?).and_return(false) expect(snippet.repository).to receive(:create_if_not_exists) + allow(snippet).to receive(:default_branch).and_return('picked') subject @@ -882,74 +893,4 @@ RSpec.describe Snippet do it_behaves_like 'can move repository storage' do let_it_be(:container) { create(:snippet, :repository) } end - - describe '#change_head_to_default_branch' do - let(:head_path) { Rails.root.join(TestEnv.repos_path, "#{snippet.disk_path}.git", 'HEAD') } - - subject { snippet.change_head_to_default_branch } - - context 'when repository does not exist' do - let(:snippet) { create(:snippet) } - - it 'does nothing' do - expect(snippet.repository_exists?).to eq false - expect(snippet.repository.raw_repository).not_to receive(:write_ref) - - subject - end - end - - context 'when repository is empty' do - let(:snippet) { create(:snippet, :empty_repo) } - - before do - allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return(default_branch) - end - - context 'when default branch in settings is different from "master"' do - let(:default_branch) { 'custom-branch' } - - it 'changes the HEAD reference to the default branch' do - expect { subject }.to change { File.read(head_path).squish }.to("ref: refs/heads/#{default_branch}") - end - end - end - - context 'when repository is not empty' do - let(:snippet) { create(:snippet, :empty_repo) } - - before do - populate_snippet_repo - end - - context 'when HEAD branch is empty' do - it 'changes HEAD to default branch' do - File.write(head_path, 'ref: refs/heads/non_existen_branch') - expect(File.read(head_path).squish).to eq 'ref: refs/heads/non_existen_branch' - - subject - - expect(File.read(head_path).squish).to eq 'ref: refs/heads/main' - expect(snippet.list_files('HEAD')).not_to be_empty - end - end - - context 'when HEAD branch is not empty' do - it 'does nothing' do - File.write(head_path, 'ref: refs/heads/main') - - expect(snippet.repository.raw_repository).not_to receive(:write_ref) - - subject - end - end - - def populate_snippet_repo - allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return('main') - - data = [{ file_path: 'new_file_test', content: 'bar' }] - snippet.snippet_repository.multi_files_action(snippet.author, data, branch_name: 'main', message: 'foo') - end - end - end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e4f25c79e53..b16a76211eb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -17,7 +17,7 @@ RSpec.describe User do it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Sortable) } it { is_expected.to include_module(TokenAuthenticatable) } - it { is_expected.to include_module(BlocksJsonSerialization) } + it { is_expected.to include_module(BlocksUnsafeSerialization) } it { is_expected.to include_module(AsyncDeviseEmail) } end @@ -116,6 +116,7 @@ RSpec.describe User do it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:pipelines) } it { is_expected.to have_many(:chat_names).dependent(:destroy) } + it { is_expected.to have_many(:saved_replies).class_name('::Users::SavedReply') } it { is_expected.to have_many(:uploads) } it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') } it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') } @@ -498,7 +499,7 @@ RSpec.describe User do end describe 'email' do - let(:expected_error) { _('is not allowed for sign-up. Check with your administrator.') } + let(:expected_error) { _('is not allowed for sign-up. Please use your regular email address. Check with your administrator.') } context 'when no signup domains allowed' do before do @@ -550,7 +551,7 @@ RSpec.describe User do user = create(:user, email: "info@test.example.com") expect { user.update!(email: "test@notexample.com") } - .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.') + .to raise_error(StandardError, 'Validation failed: Email is not allowed. Please use your regular email address. Check with your administrator.') end end @@ -623,7 +624,7 @@ RSpec.describe User do user = create(:user, email: 'info@test.com') expect { user.update!(email: 'info@example.com') } - .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.') + .to raise_error(StandardError, 'Validation failed: Email is not allowed. Please use your regular email address. Check with your administrator.') end end @@ -700,7 +701,7 @@ RSpec.describe User do user = create(:user, email: 'info@test.com') expect { user.update!(email: 'info@gitlab.com') } - .to raise_error(StandardError, 'Validation failed: Email is not allowed. Check with your administrator.') + .to raise_error(StandardError, 'Validation failed: Email is not allowed. Please use your regular email address. Check with your administrator.') end it 'does accept a valid email address' do @@ -1171,8 +1172,8 @@ RSpec.describe User do @user.update!(email: 'new_primary@example.com') @user.reload - expect(@user.emails.count).to eq 2 - expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com']) + expect(@user.emails.count).to eq 3 + expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com', 'new_primary@example.com']) end context 'when the first email was unconfirmed and the second email gets confirmed' do @@ -1593,6 +1594,66 @@ RSpec.describe User do end end + describe 'saving primary email to the emails table' do + context 'when calling skip_reconfirmation! while updating the primary email' do + let(:user) { create(:user, email: 'primary@example.com') } + + it 'adds the new email to emails' do + user.skip_reconfirmation! + user.update!(email: 'new_primary@example.com') + + expect(user.email).to eq('new_primary@example.com') + expect(user.unconfirmed_email).to be_nil + expect(user).to be_confirmed + expect(user.emails.pluck(:email)).to include('new_primary@example.com') + expect(user.emails.find_by(email: 'new_primary@example.com')).to be_confirmed + end + end + + context 'when the email is changed but not confirmed' do + let(:user) { create(:user, email: 'primary@example.com') } + + it 'does not add the new email to emails yet' do + user.update!(email: 'new_primary@example.com') + + expect(user.unconfirmed_email).to eq('new_primary@example.com') + expect(user.email).to eq('primary@example.com') + expect(user).to be_confirmed + expect(user.emails.pluck(:email)).not_to include('new_primary@example.com') + end + end + + context 'when the user is created as not confirmed' do + let(:user) { create(:user, :unconfirmed, email: 'primary@example.com') } + + it 'does not add the email to emails yet' do + expect(user).not_to be_confirmed + expect(user.emails.pluck(:email)).not_to include('primary@example.com') + end + end + + context 'when the user is created as confirmed' do + let(:user) { create(:user, email: 'primary@example.com', confirmed_at: DateTime.now.utc) } + + it 'adds the email to emails' do + expect(user).to be_confirmed + expect(user.emails.pluck(:email)).to include('primary@example.com') + end + end + + context 'when skip_confirmation! is called' do + let(:user) { build(:user, :unconfirmed, email: 'primary@example.com') } + + it 'adds the email to emails' do + user.skip_confirmation! + user.save! + + expect(user).to be_confirmed + expect(user.emails.pluck(:email)).to include('primary@example.com') + end + end + end + describe '#force_confirm' do let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days } let(:extant_confirmation_sent_at) { Date.today } @@ -3089,7 +3150,7 @@ RSpec.describe User do describe '#ldap_identity' do it 'returns ldap identity' do - user = create :omniauth_user + user = create(:omniauth_user, :ldap) expect(user.ldap_identity.provider).not_to be_empty end @@ -3717,7 +3778,7 @@ RSpec.describe User do context 'with min_access_level' do let!(:user) { create(:user) } - let!(:project) { create(:project, :private, namespace: user.namespace) } + let!(:project) { create(:project, :private, group: create(:group)) } before do project.add_developer(user) @@ -4712,6 +4773,7 @@ RSpec.describe User do expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_merge_requests_count']) expect(cache_mock).to receive(:delete).with(['users', user.id, 'review_requested_open_merge_requests_count']) + expect(cache_mock).to receive(:delete).with(['users', user.id, 'attention_requested_open_merge_requests_count']) allow(Rails).to receive(:cache).and_return(cache_mock) @@ -4719,6 +4781,20 @@ RSpec.describe User do end end + describe '#invalidate_attention_requested_count' do + let(:user) { build_stubbed(:user) } + + it 'invalidates cache for issue counter' do + cache_mock = double + + expect(cache_mock).to receive(:delete).with(['users', user.id, 'attention_requested_open_merge_requests_count']) + + allow(Rails).to receive(:cache).and_return(cache_mock) + + user.invalidate_attention_requested_count + end + end + describe '#invalidate_personal_projects_count' do let(:user) { build_stubbed(:user) } @@ -4805,6 +4881,20 @@ RSpec.describe User do end end + describe '#attention_requested_open_merge_requests_count' do + it 'returns number of open merge requests from non-archived projects' do + user = create(:user) + project = create(:project, :public) + archived_project = create(:project, :public, :archived) + + create(:merge_request, source_project: project, author: user, reviewers: [user]) + create(:merge_request, :closed, source_project: project, author: user, reviewers: [user]) + create(:merge_request, source_project: archived_project, author: user, reviewers: [user]) + + expect(user.attention_requested_open_merge_requests_count(force: true)).to eq 1 + end + end + describe '#assigned_open_issues_count' do it 'returns number of open issues from non-archived projects' do user = create(:user) diff --git a/spec/models/users/credit_card_validation_spec.rb b/spec/models/users/credit_card_validation_spec.rb index 43edf7ed093..34cfd500c26 100644 --- a/spec/models/users/credit_card_validation_spec.rb +++ b/spec/models/users/credit_card_validation_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Users::CreditCardValidation do it { is_expected.to belong_to(:user) } - it { is_expected.to validate_length_of(:holder_name).is_at_most(26) } + it { is_expected.to validate_length_of(:holder_name).is_at_most(50) } it { is_expected.to validate_length_of(:network).is_at_most(32) } it { is_expected.to validate_numericality_of(:last_digits).is_less_than_or_equal_to(9999) } diff --git a/spec/models/users/saved_reply_spec.rb b/spec/models/users/saved_reply_spec.rb new file mode 100644 index 00000000000..50138dba478 --- /dev/null +++ b/spec/models/users/saved_reply_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::SavedReply do + let_it_be(:saved_reply) { create(:saved_reply) } + + describe 'validations' do + it { is_expected.to validate_presence_of(:user_id) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:content) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to([:user_id]) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:content).is_at_most(10000) } + end +end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 699dd35196f..0016d2f517b 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -24,14 +24,6 @@ RSpec.describe WikiPage do container.wiki end - def disable_front_matter - stub_feature_flags(Gitlab::WikiPages::FrontMatterParser::FEATURE_FLAG => false) - end - - def enable_front_matter_for(thing) - stub_feature_flags(Gitlab::WikiPages::FrontMatterParser::FEATURE_FLAG => thing) - end - # Use for groups of tests that do not modify their `subject`. # # include_context 'subject is persisted page', title: 'my title' @@ -48,12 +40,6 @@ RSpec.describe WikiPage do it { expect(wiki_page).to have_attributes(front_matter: {}, content: content) } end - shared_examples 'a page with front-matter' do - let(:front_matter) { { title: 'Foo', slugs: %w[slug_a slug_b] } } - - it { expect(wiki_page.front_matter).to eq(front_matter) } - end - context 'the wiki page has front matter' do let(:content) do <<~MD @@ -68,27 +54,13 @@ RSpec.describe WikiPage do MD end - it_behaves_like 'a page with front-matter' + it 'has front-matter' do + expect(wiki_page.front_matter).to eq({ title: 'Foo', slugs: %w[slug_a slug_b] }) + end it 'strips the front matter from the content' do expect(wiki_page.content.strip).to eq('My actual content') end - - context 'the feature flag is off' do - before do - disable_front_matter - end - - it_behaves_like 'a page without front-matter' - - context 'but enabled for the container' do - before do - enable_front_matter_for(container) - end - - it_behaves_like 'a page with front-matter' - end - end end context 'the wiki page does not have front matter' do @@ -471,29 +443,6 @@ RSpec.describe WikiPage do end end - context 'the front-matter feature flag is not enabled' do - before do - disable_front_matter - end - - it 'does not update the front-matter' do - content = subject.content - subject.update(front_matter: { slugs: ['x'] }) - - page = wiki.find_page(subject.title) - - expect([subject, page]).to all(have_attributes(front_matter: be_empty, content: content)) - end - - context 'but it is enabled for the container' do - before do - enable_front_matter_for(container) - end - - it_behaves_like 'able to update front-matter' - end - end - it 'updates the wiki-page front-matter and content together' do content = 'totally new content' subject.update(content: content, front_matter: { slugs: ['x'] }) diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 2fa1abda44a..e92ae746911 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -10,4 +10,16 @@ RSpec.describe WorkItem do expect(work_item.noteable_target_type_name).to eq('issue') end end + + describe 'callbacks' do + describe 'record_create_action' do + it 'records the creation action after saving' do + expect(Gitlab::UsageDataCounters::WorkItemActivityUniqueCounter).to receive(:track_work_item_created_action) + # During the work item transition we also want to track work items as issues + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_created_action) + + create(:work_item) + end + end + end end |