diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 09:08:42 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 09:08:42 +0000 |
commit | b76ae638462ab0f673e5915986070518dd3f9ad3 (patch) | |
tree | bdab0533383b52873be0ec0eb4d3c66598ff8b91 /spec/models | |
parent | 434373eabe7b4be9593d18a585fb763f1e5f1a6f (diff) | |
download | gitlab-ce-b76ae638462ab0f673e5915986070518dd3f9ad3.tar.gz |
Add latest changes from gitlab-org/gitlab@14-2-stable-eev14.2.0-rc42
Diffstat (limited to 'spec/models')
87 files changed, 2884 insertions, 1224 deletions
diff --git a/spec/models/alert_management/alert_spec.rb b/spec/models/alert_management/alert_spec.rb index 18d486740b8..35398e29062 100644 --- a/spec/models/alert_management/alert_spec.rb +++ b/spec/models/alert_management/alert_spec.rb @@ -33,70 +33,6 @@ RSpec.describe AlertManagement::Alert do it { is_expected.to validate_length_of(:service).is_at_most(100) } it { is_expected.to validate_length_of(:monitoring_tool).is_at_most(100) } - context 'when status is triggered' do - subject { triggered_alert } - - context 'when ended_at is blank' do - it { is_expected.to be_valid } - end - - context 'when ended_at is present' do - before do - triggered_alert.ended_at = Time.current - end - - it { is_expected.to be_invalid } - end - end - - context 'when status is acknowledged' do - subject { acknowledged_alert } - - context 'when ended_at is blank' do - it { is_expected.to be_valid } - end - - context 'when ended_at is present' do - before do - acknowledged_alert.ended_at = Time.current - end - - it { is_expected.to be_invalid } - end - end - - context 'when status is resolved' do - subject { resolved_alert } - - context 'when ended_at is blank' do - before do - resolved_alert.ended_at = nil - end - - it { is_expected.to be_invalid } - end - - context 'when ended_at is present' do - it { is_expected.to be_valid } - end - end - - context 'when status is ignored' do - subject { ignored_alert } - - context 'when ended_at is blank' do - it { is_expected.to be_valid } - end - - context 'when ended_at is present' do - before do - ignored_alert.ended_at = Time.current - end - - it { is_expected.to be_invalid } - end - end - describe 'fingerprint' do let_it_be(:fingerprint) { 'fingerprint' } let_it_be(:project3, refind: true) { create(:project) } @@ -112,30 +48,30 @@ RSpec.describe AlertManagement::Alert do let_it_be(:existing_alert, refind: true) { create(:alert_management_alert, fingerprint: fingerprint, project: project3) } # We are only validating uniqueness for non-resolved alerts - where(:existing_status, :new_status, :valid) do - :resolved | :triggered | true - :resolved | :acknowledged | true - :resolved | :ignored | true - :resolved | :resolved | true - :triggered | :triggered | false - :triggered | :acknowledged | false - :triggered | :ignored | false - :triggered | :resolved | true - :acknowledged | :triggered | false - :acknowledged | :acknowledged | false - :acknowledged | :ignored | false - :acknowledged | :resolved | true - :ignored | :triggered | false - :ignored | :acknowledged | false - :ignored | :ignored | false - :ignored | :resolved | true + where(:existing_status_event, :new_status, :valid) do + :resolve | :triggered | true + :resolve | :acknowledged | true + :resolve | :ignored | true + :resolve | :resolved | true + :trigger | :triggered | false + :trigger | :acknowledged | false + :trigger | :ignored | false + :trigger | :resolved | true + :acknowledge | :triggered | false + :acknowledge | :acknowledged | false + :acknowledge | :ignored | false + :acknowledge | :resolved | true + :ignore | :triggered | false + :ignore | :acknowledged | false + :ignore | :ignored | false + :ignore | :resolved | true end with_them do let(:new_alert) { build(:alert_management_alert, new_status, fingerprint: fingerprint, project: project3) } before do - existing_alert.change_status_to(existing_status) + existing_alert.update!(status_event: existing_status_event) end if params[:valid] @@ -196,20 +132,6 @@ RSpec.describe AlertManagement::Alert do it { is_expected.to match_array(triggered_alert) } end - describe '.for_status' do - let(:status) { :resolved } - - subject { AlertManagement::Alert.for_status(status) } - - it { is_expected.to match_array(resolved_alert) } - - context 'with multiple statuses' do - let(:status) { [:resolved, :ignored] } - - it { is_expected.to match_array([resolved_alert, ignored_alert]) } - end - end - describe '.for_fingerprint' do let(:fingerprint) { SecureRandom.hex } let(:alert_with_fingerprint) { triggered_alert } @@ -302,41 +224,7 @@ RSpec.describe AlertManagement::Alert do end end - describe '.status_value' do - using RSpec::Parameterized::TableSyntax - - where(:status, :status_value) do - :triggered | 0 - :acknowledged | 1 - :resolved | 2 - :ignored | 3 - :unknown | nil - end - - with_them do - it 'returns status value by its name' do - expect(described_class.status_value(status)).to eq(status_value) - end - end - end - - describe '.status_name' do - using RSpec::Parameterized::TableSyntax - - where(:raw_status, :status) do - 0 | :triggered - 1 | :acknowledged - 2 | :resolved - 3 | :ignored - -1 | nil - end - - with_them do - it 'returns status name by its values' do - expect(described_class.status_name(raw_status)).to eq(status) - end - end - end + it_behaves_like 'a model including Escalatable' describe '.counts_by_status' do subject { described_class.counts_by_status } @@ -454,85 +342,17 @@ RSpec.describe AlertManagement::Alert do end end - describe '#to_reference' do - it { expect(triggered_alert.to_reference).to eq("^alert##{triggered_alert.iid}") } - end - - describe '#trigger' do - subject { alert.trigger } - - context 'when alert is in triggered state' do - let(:alert) { triggered_alert } - - it 'does not change the alert status' do - expect { subject }.not_to change { alert.reload.status } - end - end - - context 'when alert not in triggered state' do - let(:alert) { resolved_alert } - - it 'changes the alert status to triggered' do - expect { subject }.to change { alert.triggered? }.to(true) - end - - it 'resets ended at' do - expect { subject }.to change { alert.reload.ended_at }.to nil - end - end - end - - describe '#acknowledge' do - subject { alert.acknowledge } - - let(:alert) { resolved_alert } - - it 'changes the alert status to acknowledged' do - expect { subject }.to change { alert.acknowledged? }.to(true) - end - - it 'resets ended at' do - expect { subject }.to change { alert.reload.ended_at }.to nil - end - end - - describe '#resolve' do - let!(:ended_at) { Time.current } - - subject do - alert.ended_at = ended_at - alert.resolve - end - - context 'when alert already resolved' do - let(:alert) { resolved_alert } - - it 'does not change the alert status' do - expect { subject }.not_to change { resolved_alert.reload.status } - end - end - - context 'when alert is not resolved' do - let(:alert) { triggered_alert } - - it 'changes alert status to "resolved"' do - expect { subject }.to change { alert.resolved? }.to(true) - end + describe '#open?' do + it 'returns true when the status is open status' do + expect(triggered_alert.open?).to be true + expect(acknowledged_alert.open?).to be true + expect(resolved_alert.open?).to be false + expect(ignored_alert.open?).to be false end end - describe '#ignore' do - subject { alert.ignore } - - let(:alert) { resolved_alert } - - it 'changes the alert status to ignored' do - expect { subject }.to change { alert.ignored? }.to(true) - end - - it 'resets ended at' do - expect { subject }.to change { alert.reload.ended_at }.to nil - end + describe '#to_reference' do + it { expect(triggered_alert.to_reference).to eq("^alert##{triggered_alert.iid}") } end describe '#register_new_event!' do @@ -545,53 +365,20 @@ RSpec.describe AlertManagement::Alert do end end - describe '#status_event_for' do - using RSpec::Parameterized::TableSyntax - - where(:for_status, :event) do - :triggered | :trigger - 'triggered' | :trigger - :acknowledged | :acknowledge - 'acknowledged' | :acknowledge - :resolved | :resolve - 'resolved' | :resolve - :ignored | :ignore - 'ignored' | :ignore - :unknown | nil - nil | nil - '' | nil - 1 | nil - end + describe '#resolved_at' do + subject { resolved_alert.resolved_at } - with_them do - let(:alert) { build(:alert_management_alert, project: project) } - - it 'returns event by status name' do - expect(alert.status_event_for(for_status)).to eq(event) - end - end + it { is_expected.to eq(resolved_alert.ended_at) } end - describe '#change_status_to' do - let_it_be_with_reload(:alert) { create(:alert_management_alert, project: project) } + describe '#resolved_at=' do + let(:resolve_time) { Time.current } - context 'with valid statuses' do - it 'changes the status to triggered' do - alert.acknowledge! # change to non-triggered status - expect { alert.change_status_to(:triggered) }.to change { alert.triggered? }.to(true) - end + it 'sets ended_at' do + triggered_alert.resolved_at = resolve_time - %i(acknowledged resolved ignored).each do |status| - it "changes the status to #{status}" do - expect { alert.change_status_to(status) }.to change { alert.public_send(:"#{status}?") }.to(true) - end - end - end - - context 'with invalid status' do - it 'does not change the current status' do - expect { alert.change_status_to(nil) }.not_to change { alert.status } - end + expect(triggered_alert.ended_at).to eq(resolve_time) + expect(triggered_alert.resolved_at).to eq(resolve_time) end end end diff --git a/spec/models/analytics/cycle_analytics/stage_event_hash_spec.rb b/spec/models/analytics/cycle_analytics/stage_event_hash_spec.rb new file mode 100644 index 00000000000..ffddaf1e1b2 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/stage_event_hash_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Analytics::CycleAnalytics::StageEventHash, type: :model do + let(:stage_event_hash) { described_class.create!(hash_sha256: hash_sha256) } + let(:hash_sha256) { 'does_not_matter' } + + describe 'associations' do + it { is_expected.to have_many(:cycle_analytics_project_stages) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:hash_sha256) } + end + + describe '.record_id_by_hash_sha256' do + it 'returns an existing id' do + id = stage_event_hash.id + same_id = described_class.record_id_by_hash_sha256(hash_sha256) + + expect(same_id).to eq(id) + end + + it 'creates a new record' do + expect do + described_class.record_id_by_hash_sha256(hash_sha256) + end.to change { described_class.count }.from(0).to(1) + end + end + + describe '.cleanup_if_unused' do + it 'removes the record' do + described_class.cleanup_if_unused(stage_event_hash.id) + + expect(described_class.find_by_id(stage_event_hash.id)).to be_nil + end + + it 'does not remove the record' do + id = create(:cycle_analytics_project_stage).stage_event_hash_id + + described_class.cleanup_if_unused(id) + + expect(described_class.find_by_id(id)).not_to be_nil + end + end +end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 85a6717d259..f9a05c720a3 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -39,13 +39,14 @@ RSpec.describe ApplicationRecord do let(:suggestion_attributes) { attributes_for(:suggestion).merge!(note_id: note.id) } - describe '.safe_find_or_create_by' do + shared_examples '.safe_find_or_create_by' do it 'creates the suggestion avoiding race conditions' do - expect(Suggestion).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) - allow(Suggestion).to receive(:find_or_create_by).and_call_original + existing_suggestion = double(:Suggestion) - expect { Suggestion.safe_find_or_create_by(suggestion_attributes) } - .to change { Suggestion.count }.by(1) + expect(Suggestion).to receive(:find_by).and_return(nil, existing_suggestion) + expect(Suggestion).to receive(:create).and_raise(ActiveRecord::RecordNotUnique) + + expect(Suggestion.safe_find_or_create_by(suggestion_attributes)).to eq(existing_suggestion) end it 'passes a block to find_or_create_by' do @@ -62,10 +63,8 @@ RSpec.describe ApplicationRecord do end end - describe '.safe_find_or_create_by!' do + shared_examples '.safe_find_or_create_by!' do it 'creates a record using safe_find_or_create_by' do - expect(Suggestion).to receive(:find_or_create_by).and_call_original - expect(Suggestion.safe_find_or_create_by!(suggestion_attributes)) .to be_a(Suggestion) end @@ -89,6 +88,24 @@ RSpec.describe ApplicationRecord do .to raise_error(ActiveRecord::RecordNotFound) end end + + context 'when optimized_safe_find_or_create_by is enabled' do + before do + stub_feature_flags(optimized_safe_find_or_create_by: true) + end + + it_behaves_like '.safe_find_or_create_by' + it_behaves_like '.safe_find_or_create_by!' + end + + context 'when optimized_safe_find_or_create_by is disabled' do + before do + stub_feature_flags(optimized_safe_find_or_create_by: false) + end + + it_behaves_like '.safe_find_or_create_by' + it_behaves_like '.safe_find_or_create_by!' + end end describe '.underscore' do @@ -105,6 +122,50 @@ RSpec.describe ApplicationRecord do end end + describe '.transaction', :delete do + it 'opens a new transaction' do + expect(described_class.connection.transaction_open?).to be false + + Project.transaction do + expect(Project.connection.transaction_open?).to be true + + Project.transaction(requires_new: true) do + expect(Project.connection.transaction_open?).to be true + end + end + end + + it 'does not increment a counter when a transaction is not nested' do + expect(described_class.connection.transaction_open?).to be false + + expect(::Gitlab::Database::Metrics) + .not_to receive(:subtransactions_increment) + + Project.transaction do + expect(Project.connection.transaction_open?).to be true + end + + Project.transaction(requires_new: true) do + expect(Project.connection.transaction_open?).to be true + end + end + + it 'increments a counter when a nested transaction is created' do + expect(described_class.connection.transaction_open?).to be false + + expect(::Gitlab::Database::Metrics) + .to receive(:subtransactions_increment) + .with('Project') + .once + + Project.transaction do + Project.transaction(requires_new: true) do + expect(Project.connection.transaction_open?).to be true + end + end + end + end + describe '.with_fast_read_statement_timeout' do context 'when the query runs faster than configured timeout' do it 'executes the query without error' do diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 80471a09bbd..e9c5ffef210 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -218,6 +218,16 @@ RSpec.describe ApplicationSetting do end end + describe 'default_branch_name validaitions' do + context "when javascript tags get sanitized properly" do + it "gets sanitized properly" do + setting.update!(default_branch_name: "hello<script>alert(1)</script>") + + expect(setting.default_branch_name).to eq('hello') + end + end + end + describe 'spam_check_endpoint' do context 'when spam_check_endpoint is enabled' do before do @@ -834,6 +844,23 @@ RSpec.describe ApplicationSetting do end end + describe '#customers_dot_jwt_signing_key' do + it { is_expected.not_to allow_value('').for(:customers_dot_jwt_signing_key) } + it { is_expected.not_to allow_value('invalid RSA key').for(:customers_dot_jwt_signing_key) } + it { is_expected.to allow_value(nil).for(:customers_dot_jwt_signing_key) } + it { is_expected.to allow_value(OpenSSL::PKey::RSA.new(1024).to_pem).for(:customers_dot_jwt_signing_key) } + + it 'is encrypted' do + subject.customers_dot_jwt_signing_key = OpenSSL::PKey::RSA.new(1024).to_pem + + aggregate_failures do + expect(subject.encrypted_customers_dot_jwt_signing_key).to be_present + expect(subject.encrypted_customers_dot_jwt_signing_key_iv).to be_present + expect(subject.encrypted_customers_dot_jwt_signing_key).not_to eq(subject.customers_dot_jwt_signing_key) + end + end + end + describe '#cloud_license_auth_token' do it { is_expected.to allow_value(nil).for(:cloud_license_auth_token) } @@ -927,7 +954,7 @@ RSpec.describe ApplicationSetting do context 'when ApplicationSettings does not have a primary key' do before do - allow(ActiveRecord::Base.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil) + allow(described_class.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil) end it 'raises an exception' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 0c344270e0b..26abc98656e 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -20,7 +20,6 @@ RSpec.describe Ci::Build do it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } - it { is_expected.to have_many(:trace_sections) } it { is_expected.to have_many(:needs) } it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:job_variables) } @@ -29,6 +28,7 @@ RSpec.describe Ci::Build do it { is_expected.to have_one(:deployment) } it { is_expected.to have_one(:runner_session) } + it { is_expected.to have_one(:trace_metadata) } it { is_expected.to validate_presence_of(:ref) } @@ -345,12 +345,9 @@ RSpec.describe Ci::Build do end describe '#stick_build_if_status_changed' do - it 'sticks the build if the status changed' do + it 'sticks the build if the status changed', :db_load_balancing do job = create(:ci_build, :pending) - allow(Gitlab::Database::LoadBalancing).to receive(:enable?) - .and_return(true) - expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) .with(:build, job.id) @@ -1105,17 +1102,6 @@ RSpec.describe Ci::Build do end end - describe '#parse_trace_sections!' do - it 'calls ExtractSectionsFromBuildTraceService' do - expect(Ci::ExtractSectionsFromBuildTraceService) - .to receive(:new).with(project, build.user).once.and_call_original - expect_any_instance_of(Ci::ExtractSectionsFromBuildTraceService) - .to receive(:execute).with(build).once - - build.parse_trace_sections! - end - end - describe '#trace' do subject { build.trace } @@ -1955,17 +1941,7 @@ RSpec.describe Ci::Build do described_class.retry(build, user) end - context 'when prevent_retry_of_retried_jobs feature flag is enabled' do - it { is_expected.not_to be_retryable } - end - - context 'when prevent_retry_of_retried_jobs feature flag is disabled' do - before do - stub_feature_flags(prevent_retry_of_retried_jobs: false) - end - - it { is_expected.to be_retryable } - end + it { is_expected.not_to be_retryable } end end end @@ -2214,34 +2190,12 @@ RSpec.describe Ci::Build do expect(build.options['image']).to be_nil end - context 'when ci_build_metadata_config is set' do - before do - stub_feature_flags(ci_build_metadata_config: true) - end - - it 'persist data in build metadata' do - expect(build.metadata.read_attribute(:config_options)).to eq(options.symbolize_keys) - end - - it 'does not persist data in build' do - expect(build.read_attribute(:options)).to be_nil - end + it 'persist data in build metadata' do + expect(build.metadata.read_attribute(:config_options)).to eq(options.symbolize_keys) end - context 'when ci_build_metadata_config is disabled' do - let(:build) { create(:ci_build, pipeline: pipeline) } - - before do - stub_feature_flags(ci_build_metadata_config: false) - end - - it 'persist data in build' do - expect(build.read_attribute(:options)).to eq(options.symbolize_keys) - end - - it 'does not persist data in build metadata' do - expect(build.metadata.read_attribute(:config_options)).to be_nil - end + it 'does not persist data in build' do + expect(build.read_attribute(:options)).to be_nil end context 'when options include artifacts:expose_as' do @@ -2668,6 +2622,7 @@ RSpec.describe Ci::Build do { key: 'CI_PROJECT_URL', value: project.web_url, public: true, masked: false }, { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true, masked: false }, { key: 'CI_PROJECT_REPOSITORY_LANGUAGES', value: project.repository_languages.map(&:name).join(',').downcase, public: true, masked: false }, + { key: 'CI_PROJECT_CLASSIFICATION_LABEL', value: project.external_authorization_classification_label, public: true, masked: false }, { key: 'CI_DEFAULT_BRANCH', value: project.default_branch, public: true, masked: false }, { key: 'CI_CONFIG_PATH', value: project.ci_config_path_or_default, public: true, masked: false }, { key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true, masked: false }, @@ -3195,6 +3150,17 @@ RSpec.describe Ci::Build do end context 'when container registry is enabled' do + let_it_be_with_reload(:project) { create(:project, :public, :repository, group: group) } + + let_it_be_with_reload(:pipeline) do + create(:ci_pipeline, project: project, + sha: project.commit.id, + ref: project.default_branch, + status: 'success') + end + + let_it_be_with_refind(:build) { create(:ci_build, pipeline: pipeline) } + let(:container_registry_enabled) { true } let(:ci_registry) do { key: 'CI_REGISTRY', value: 'registry.example.com', public: true, masked: false } @@ -3206,7 +3172,7 @@ RSpec.describe Ci::Build do context 'and is disabled for project' do before do - project.update!(container_registry_enabled: false) + project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) end it { is_expected.to include(ci_registry) } @@ -3215,7 +3181,16 @@ RSpec.describe Ci::Build do context 'and is enabled for project' do before do - project.update!(container_registry_enabled: true) + project.project_feature.update_column(:container_registry_access_level, ProjectFeature::ENABLED) + end + + it { is_expected.to include(ci_registry) } + it { is_expected.to include(ci_registry_image) } + end + + context 'and is private for project' do + before do + project.project_feature.update_column(:container_registry_access_level, ProjectFeature::PRIVATE) end it { is_expected.to include(ci_registry) } @@ -3613,36 +3588,14 @@ RSpec.describe Ci::Build do end end - context 'when ci_build_metadata_config is set' do - before do - stub_feature_flags(ci_build_metadata_config: true) - end - - it_behaves_like 'having consistent representation' - - it 'persist data in build metadata' do - expect(build.metadata.read_attribute(:config_variables)).not_to be_nil - end + it_behaves_like 'having consistent representation' - it 'does not persist data in build' do - expect(build.read_attribute(:yaml_variables)).to be_nil - end + it 'persist data in build metadata' do + expect(build.metadata.read_attribute(:config_variables)).not_to be_nil end - context 'when ci_build_metadata_config is disabled' do - before do - stub_feature_flags(ci_build_metadata_config: false) - end - - it_behaves_like 'having consistent representation' - - it 'persist data in build' do - expect(build.read_attribute(:yaml_variables)).not_to be_nil - end - - it 'does not persist data in build metadata' do - expect(build.metadata.read_attribute(:config_variables)).to be_nil - end + it 'does not persist data in build' do + expect(build.read_attribute(:yaml_variables)).to be_nil end end @@ -3727,7 +3680,7 @@ RSpec.describe Ci::Build do it 'ensures that it is not run in database transaction' do expect(job.pipeline.persistent_ref).to receive(:create) do - expect(Gitlab::Database).not_to be_inside_transaction + expect(Gitlab::Database.main).not_to be_inside_transaction end run_job_without_exception @@ -3792,7 +3745,21 @@ RSpec.describe Ci::Build do context 'when artifacts of depended job has been expired' do let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } - it { expect(job).not_to have_valid_build_dependencies } + context 'when pipeline is not locked' do + before do + build.pipeline.unlocked! + end + + it { expect(job).not_to have_valid_build_dependencies } + end + + context 'when pipeline is locked' do + before do + build.pipeline.artifacts_locked! + end + + it { expect(job).to have_valid_build_dependencies } + end end context 'when artifacts of depended job has been erased' do @@ -4788,51 +4755,21 @@ RSpec.describe Ci::Build do subject { build.send(:write_metadata_attribute, :options, :config_options, options) } - context 'when ci_build_metadata_config is set' do + context 'when data in build is already set' do before do - stub_feature_flags(ci_build_metadata_config: true) + build.write_attribute(:options, existing_options) end - context 'when data in build is already set' do - before do - build.write_attribute(:options, existing_options) - end - - it 'does set metadata options' do - subject - - expect(build.metadata.read_attribute(:config_options)).to eq(options) - end - - it 'does reset build options' do - subject - - expect(build.read_attribute(:options)).to be_nil - end - end - end + it 'does set metadata options' do + subject - context 'when ci_build_metadata_config is disabled' do - before do - stub_feature_flags(ci_build_metadata_config: false) + expect(build.metadata.read_attribute(:config_options)).to eq(options) end - context 'when data in build metadata is already set' do - before do - build.ensure_metadata.write_attribute(:config_options, existing_options) - end - - it 'does set metadata options' do - subject - - expect(build.read_attribute(:options)).to eq(options) - end - - it 'does reset build options' do - subject + it 'does reset build options' do + subject - expect(build.metadata.read_attribute(:config_options)).to be_nil - end + expect(build.read_attribute(:options)).to be_nil end end end @@ -4842,8 +4779,24 @@ RSpec.describe Ci::Build do let!(:pre_stage_job_invalid) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test2', stage_idx: 1) } let!(:job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 2, options: { dependencies: %w(test1 test2) }) } - it 'returns invalid dependencies' do - expect(job.invalid_dependencies).to eq([pre_stage_job_invalid]) + context 'when pipeline is locked' do + before do + build.pipeline.unlocked! + end + + it 'returns invalid dependencies when expired' do + expect(job.invalid_dependencies).to eq([pre_stage_job_invalid]) + end + end + + context 'when pipeline is not locked' do + before do + build.pipeline.artifacts_locked! + end + + it 'returns no invalid dependencies when expired' do + expect(job.invalid_dependencies).to eq([]) + end end end @@ -5267,6 +5220,14 @@ RSpec.describe Ci::Build do end end + describe '.with_project_and_metadata' do + it 'does not join across databases' do + with_cross_joins_prevented do + ::Ci::Build.with_project_and_metadata.to_a + end + end + end + describe '.without_coverage' do let!(:build_with_coverage) { create(:ci_build, pipeline: pipeline, coverage: 100.0) } diff --git a/spec/models/ci/build_trace_metadata_spec.rb b/spec/models/ci/build_trace_metadata_spec.rb new file mode 100644 index 00000000000..42b9d5d34b6 --- /dev/null +++ b/spec/models/ci/build_trace_metadata_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::BuildTraceMetadata do + it { is_expected.to belong_to(:build) } + it { is_expected.to belong_to(:trace_artifact) } + + it { is_expected.to validate_presence_of(:build) } +end diff --git a/spec/models/ci/build_trace_section_name_spec.rb b/spec/models/ci/build_trace_section_name_spec.rb deleted file mode 100644 index b220e67d48e..00000000000 --- a/spec/models/ci/build_trace_section_name_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::BuildTraceSectionName, model: true do - subject { build(:ci_build_trace_section_name) } - - it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:trace_sections)} - - it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } -end diff --git a/spec/models/ci/build_trace_section_spec.rb b/spec/models/ci/build_trace_section_spec.rb deleted file mode 100644 index 640bd202b3a..00000000000 --- a/spec/models/ci/build_trace_section_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::BuildTraceSection, model: true do - it { is_expected.to belong_to(:build)} - it { is_expected.to belong_to(:project)} - it { is_expected.to belong_to(:section_name)} - - it { is_expected.to validate_presence_of(:section_name) } - it { is_expected.to validate_presence_of(:build) } - it { is_expected.to validate_presence_of(:project) } -end diff --git a/spec/models/ci/build_trace_spec.rb b/spec/models/ci/build_trace_spec.rb index 3beca0565c6..bd24e8be1ac 100644 --- a/spec/models/ci/build_trace_spec.rb +++ b/spec/models/ci/build_trace_spec.rb @@ -32,4 +32,14 @@ RSpec.describe Ci::BuildTrace do { offset: 0, content: [{ text: 'the-stream' }] } ]) end + + context 'with invalid UTF-8 data' do + let(:data) { StringIO.new("UTF-8 dashes here: ───\n🐤🐤🐤🐤\xF0\x9F\x90\n") } + + it 'returns valid UTF-8 data', :aggregate_failures do + expect(subject.lines[0]).to eq({ offset: 0, content: [{ text: 'UTF-8 dashes here: ───' }] } ) + # Each of the dashes is 3 bytes, so we get 19 + 9 + 1 = 29 + expect(subject.lines[1]).to eq({ offset: 29, content: [{ text: '🐤🐤🐤🐤�' }] } ) + end + end end diff --git a/spec/models/ci/pending_build_spec.rb b/spec/models/ci/pending_build_spec.rb index b64f3999232..0518c9a1652 100644 --- a/spec/models/ci/pending_build_spec.rb +++ b/spec/models/ci/pending_build_spec.rb @@ -8,6 +8,34 @@ RSpec.describe Ci::PendingBuild do let(:build) { create(:ci_build, :created, pipeline: pipeline) } + describe 'associations' do + it { is_expected.to belong_to :project } + it { is_expected.to belong_to :build } + it { is_expected.to belong_to :namespace } + end + + describe 'scopes' do + describe '.with_instance_runners' do + subject(:pending_builds) { described_class.with_instance_runners } + + let!(:pending_build_1) { create(:ci_pending_build, instance_runners_enabled: false) } + + context 'when pending builds cannot be picked up by runner' do + it 'returns an empty collection of pending builds' do + expect(pending_builds).to be_empty + end + end + + context 'when pending builds can be picked up by runner' do + let!(:pending_build_2) { create(:ci_pending_build) } + + it 'returns matching pending builds' do + expect(pending_builds).to contain_exactly(pending_build_2) + end + end + end + end + describe '.upsert_from_build!' do context 'another pending entry does not exist' do it 'creates a new pending entry' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 74a476a6422..da89eccc3b2 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -263,6 +263,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '.with_pipeline_source' do + subject { described_class.with_pipeline_source(source) } + + let(:source) { 'web' } + + let_it_be(:push_pipeline) { create(:ci_pipeline, source: :push) } + let_it_be(:web_pipeline) { create(:ci_pipeline, source: :web) } + let_it_be(:api_pipeline) { create(:ci_pipeline, source: :api) } + + it 'contains pipelines created due to specified source' do + expect(subject).to contain_exactly(web_pipeline) + end + end + describe '.ci_sources' do subject { described_class.ci_sources } @@ -2263,18 +2277,38 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '.latest_successful_for_refs' do - let!(:latest_successful_pipeline1) do - create_pipeline(:success, 'ref1', 'D') - end + subject(:latest_successful_for_refs) { described_class.latest_successful_for_refs(refs) } - let!(:latest_successful_pipeline2) do - create_pipeline(:success, 'ref2', 'D') + context 'when refs are specified' do + let(:refs) { %w(first_ref second_ref third_ref) } + + before do + create(:ci_empty_pipeline, id: 1001, status: :success, ref: 'first_ref', sha: 'sha') + create(:ci_empty_pipeline, id: 1002, status: :success, ref: 'second_ref', sha: 'sha') + end + + let!(:latest_successful_pipeline_for_first_ref) do + create(:ci_empty_pipeline, id: 2001, status: :success, ref: 'first_ref', sha: 'sha') + end + + let!(:latest_successful_pipeline_for_second_ref) do + create(:ci_empty_pipeline, id: 2002, status: :success, ref: 'second_ref', sha: 'sha') + end + + it 'returns the latest successful pipeline for both refs' do + expect(latest_successful_for_refs).to eq({ + 'first_ref' => latest_successful_pipeline_for_first_ref, + 'second_ref' => latest_successful_pipeline_for_second_ref + }) + end end - it 'returns the latest successful pipeline for both refs' do - refs = %w(ref1 ref2 ref3) + context 'when no refs are specified' do + let(:refs) { [] } - expect(described_class.latest_successful_for_refs(refs)).to eq({ 'ref1' => latest_successful_pipeline1, 'ref2' => latest_successful_pipeline2 }) + it 'returns an empty relation whenno refs are specified' do + expect(latest_successful_for_refs).to be_empty + end end end end diff --git a/spec/models/ci/resource_spec.rb b/spec/models/ci/resource_spec.rb index 5574f6f82b2..e883d704768 100644 --- a/spec/models/ci/resource_spec.rb +++ b/spec/models/ci/resource_spec.rb @@ -15,6 +15,22 @@ RSpec.describe Ci::Resource do end end + describe '.retained' do + subject { described_class.retained } + + it "returns the resource if it's retained" do + resource = create(:ci_resource, processable: create(:ci_build)) + + is_expected.to eq([resource]) + end + + it "returns empty if it's not retained" do + create(:ci_resource, processable: nil) + + is_expected.to be_empty + end + end + describe '.retained_by' do subject { described_class.retained_by(build) } @@ -25,4 +41,40 @@ RSpec.describe Ci::Resource do is_expected.to eq([resource]) end end + + describe '.stale_processables' do + subject { resource_group.resources.stale_processables } + + let!(:resource_group) { create(:ci_resource_group) } + let!(:resource) { create(:ci_resource, processable: build, resource_group: resource_group) } + + context 'when the processable is running' do + let!(:build) { create(:ci_build, :running, resource_group: resource_group) } + + before do + # Creating unrelated builds to make sure the `retained` scope is working + create(:ci_build, :running, resource_group: resource_group) + end + + it 'returns empty' do + is_expected.to be_empty + end + + context 'and doomed' do + before do + build.doom! + end + + it 'returns empty' do + is_expected.to be_empty + end + + it 'returns the stale prosessable a few minutes later' do + travel_to(10.minutes.since) do + is_expected.to eq([build]) + end + end + end + end + end end diff --git a/spec/models/ci/runner_namespace_spec.rb b/spec/models/ci/runner_namespace_spec.rb index 41d805adb9f..4e7cf7a3cb3 100644 --- a/spec/models/ci/runner_namespace_spec.rb +++ b/spec/models/ci/runner_namespace_spec.rb @@ -4,6 +4,12 @@ require 'spec_helper' RSpec.describe Ci::RunnerNamespace do it_behaves_like 'includes Limitable concern' do + before do + skip_default_enabled_yaml_check + + stub_feature_flags(ci_runner_limits_override: false) + end + subject { build(:ci_runner_namespace, group: create(:group, :nested), runner: create(:ci_runner, :group)) } end end diff --git a/spec/models/ci/runner_project_spec.rb b/spec/models/ci/runner_project_spec.rb index 13369dba2cf..fef1416a84a 100644 --- a/spec/models/ci/runner_project_spec.rb +++ b/spec/models/ci/runner_project_spec.rb @@ -4,6 +4,12 @@ require 'spec_helper' RSpec.describe Ci::RunnerProject do it_behaves_like 'includes Limitable concern' do + before do + skip_default_enabled_yaml_check + + stub_feature_flags(ci_runner_limits_override: false) + end + subject { build(:ci_runner_project, project: create(:project), runner: create(:ci_runner, :project)) } end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 69b4d752f4c..a951af4cc4f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -79,6 +79,15 @@ RSpec.describe CommitStatus do end end + describe '.updated_at_before' do + it 'finds the relevant records' do + status = create(:commit_status, updated_at: 1.day.ago, project: project) + create(:commit_status, updated_at: 1.day.since, project: project) + + expect(described_class.updated_at_before(Time.current)).to eq([status]) + end + end + describe '.updated_before' do let!(:lookback) { 5.days.ago } let!(:timeout) { 1.day.ago } diff --git a/spec/models/concerns/case_sensitivity_spec.rb b/spec/models/concerns/case_sensitivity_spec.rb index 7cf7b825d7d..269f9577267 100644 --- a/spec/models/concerns/case_sensitivity_spec.rb +++ b/spec/models/concerns/case_sensitivity_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe CaseSensitivity do describe '.iwhere' do - let_it_be(:connection) { ActiveRecord::Base.connection } + let_it_be(:connection) { Namespace.connection } let_it_be(:model) do Class.new(ActiveRecord::Base) do include CaseSensitivity diff --git a/spec/models/concerns/ci/has_status_spec.rb b/spec/models/concerns/ci/has_status_spec.rb index b16420bc658..0709a050056 100644 --- a/spec/models/concerns/ci/has_status_spec.rb +++ b/spec/models/concerns/ci/has_status_spec.rb @@ -351,6 +351,18 @@ RSpec.describe Ci::HasStatus do it_behaves_like 'not containing the job', status end end + + describe '.complete' do + subject { CommitStatus.complete } + + described_class::COMPLETED_STATUSES.each do |status| + it_behaves_like 'containing the job', status + end + + described_class::ACTIVE_STATUSES.each do |status| + it_behaves_like 'not containing the job', status + end + end end describe '::DEFAULT_STATUS' do diff --git a/spec/models/concerns/each_batch_spec.rb b/spec/models/concerns/each_batch_spec.rb index 5f4e5d4bd98..f1fb4fcbd03 100644 --- a/spec/models/concerns/each_batch_spec.rb +++ b/spec/models/concerns/each_batch_spec.rb @@ -9,6 +9,8 @@ RSpec.describe EachBatch do include EachBatch self.table_name = 'users' + + scope :never_signed_in, -> { where(sign_in_count: 0) } end end @@ -72,5 +74,16 @@ RSpec.describe EachBatch do expect(ids).to eq(ids.sort.reverse) end + + describe 'current scope' do + let(:entry) { create(:user, sign_in_count: 1) } + let(:ids_with_new_relation) { model.where(id: entry.id).pluck(:id) } + + it 'does not leak current scope to block being executed' do + model.never_signed_in.each_batch(of: 5) do |relation| + expect(ids_with_new_relation).to include(entry.id) + end + end + end end end diff --git a/spec/models/concerns/has_integrations_spec.rb b/spec/models/concerns/has_integrations_spec.rb index 6b3f75bfcfd..ea6b0e69209 100644 --- a/spec/models/concerns/has_integrations_spec.rb +++ b/spec/models/concerns/has_integrations_spec.rb @@ -17,14 +17,6 @@ RSpec.describe HasIntegrations do create(:integrations_slack, project: project_4, inherit_from_id: nil) end - describe '.with_custom_integration_for' do - it 'returns projects with custom integrations' do - # We use pagination to verify that the group is excluded from the query - expect(Project.with_custom_integration_for(instance_integration, 0, 2)).to contain_exactly(project_2, project_3) - expect(Project.with_custom_integration_for(instance_integration)).to contain_exactly(project_2, project_3) - end - end - describe '.without_integration' do it 'returns projects without integration' do expect(Project.without_integration(instance_integration)).to contain_exactly(project_4) diff --git a/spec/models/concerns/limitable_spec.rb b/spec/models/concerns/limitable_spec.rb index 6b25ed39efb..850282d54c7 100644 --- a/spec/models/concerns/limitable_spec.rb +++ b/spec/models/concerns/limitable_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' -require 'active_model' +require 'spec_helper' RSpec.describe Limitable do let(:minimal_test_class) do @@ -17,7 +16,7 @@ RSpec.describe Limitable do end before do - stub_const("MinimalTestClass", minimal_test_class) + stub_const('MinimalTestClass', minimal_test_class) end it { expect(MinimalTestClass.limit_name).to eq('test_classes') } @@ -37,25 +36,50 @@ RSpec.describe Limitable do instance.valid?(:create) end - context 'with custom relation' do - before do - MinimalTestClass.limit_relation = :custom_relation + context 'with custom relation and feature flags' do + using RSpec::Parameterized::TableSyntax + + where(:limit_feature_flag, :limit_feature_flag_value, :limit_feature_flag_for_override, :limit_feature_flag_override_value, :expect_limit_applied?) do + nil | nil | nil | nil | true + :some_feature_flag | false | nil | nil | false + :some_feature_flag | true | nil | nil | true + :some_feature_flag | true | :some_feature_flag_disable | false | true + :some_feature_flag | false | :some_feature_flag_disable | false | false + :some_feature_flag | false | :some_feature_flag_disable | true | false + :some_feature_flag | true | :some_feature_flag_disable | true | false end - it 'triggers custom limit_relation' do - instance = MinimalTestClass.new + with_them do + let(:instance) { MinimalTestClass.new } - def instance.project - @project ||= Object.new - end + before do + def instance.project + @project ||= stub_feature_flag_gate('CustomActor') + end + + stub_feature_flags("#{limit_feature_flag}": limit_feature_flag_value ? [instance.project] : false) if limit_feature_flag + stub_feature_flags("#{limit_feature_flag_for_override}": limit_feature_flag_override_value ? [instance.project] : false) if limit_feature_flag_for_override + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check - limits = Object.new - custom_relation = Object.new - expect(instance).to receive(:custom_relation).and_return(custom_relation) - expect(instance.project).to receive(:actual_limits).and_return(limits) - expect(limits).to receive(:exceeded?).with(instance.class.name.demodulize.tableize, custom_relation).and_return(false) + MinimalTestClass.limit_relation = :custom_relation + MinimalTestClass.limit_feature_flag = limit_feature_flag + MinimalTestClass.limit_feature_flag_for_override = limit_feature_flag_for_override + end - instance.valid?(:create) + it 'acts according to the feature flag settings' do + limits = Object.new + custom_relation = Object.new + if expect_limit_applied? + expect(instance).to receive(:custom_relation).and_return(custom_relation) + expect(instance.project).to receive(:actual_limits).and_return(limits) + expect(limits).to receive(:exceeded?).with(instance.class.name.demodulize.tableize, custom_relation).and_return(false) + else + expect(instance).not_to receive(:custom_relation) + end + + instance.valid?(:create) + end end end end diff --git a/spec/models/concerns/sortable_spec.rb b/spec/models/concerns/sortable_spec.rb index cfa00bab025..f1ae89f33af 100644 --- a/spec/models/concerns/sortable_spec.rb +++ b/spec/models/concerns/sortable_spec.rb @@ -70,8 +70,8 @@ RSpec.describe Sortable do it 'ascending' do expect(relation).to receive(:reorder).once.and_call_original - table = Regexp.escape(ActiveRecord::Base.connection.quote_table_name(:namespaces)) - column = Regexp.escape(ActiveRecord::Base.connection.quote_column_name(:name)) + table = Regexp.escape(ApplicationRecord.connection.quote_table_name(:namespaces)) + column = Regexp.escape(ApplicationRecord.connection.quote_column_name(:name)) sql = relation.order_by('name_asc').to_sql @@ -81,8 +81,8 @@ RSpec.describe Sortable do it 'descending' do expect(relation).to receive(:reorder).once.and_call_original - table = Regexp.escape(ActiveRecord::Base.connection.quote_table_name(:namespaces)) - column = Regexp.escape(ActiveRecord::Base.connection.quote_column_name(:name)) + table = Regexp.escape(ApplicationRecord.connection.quote_table_name(:namespaces)) + column = Regexp.escape(ApplicationRecord.connection.quote_column_name(:name)) sql = relation.order_by('name_desc').to_sql diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 3c5f3b2d2ad..5edaab56e2d 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -28,11 +28,11 @@ RSpec.describe Spammable do it 'returns true for public project' do issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) - expect(issue.check_for_spam?).to eq(true) + expect(issue.check_for_spam?(user: issue.author)).to eq(true) end it 'returns false for other visibility levels' do - expect(issue.check_for_spam?).to eq(false) + expect(issue.check_for_spam?(user: issue.author)).to eq(false) end end diff --git a/spec/models/concerns/strip_attribute_spec.rb b/spec/models/concerns/strip_attribute_spec.rb index 812f0a015f7..4357bc93361 100644 --- a/spec/models/concerns/strip_attribute_spec.rb +++ b/spec/models/concerns/strip_attribute_spec.rb @@ -5,12 +5,12 @@ require 'spec_helper' RSpec.describe StripAttribute do let(:milestone) { create(:milestone) } - describe ".strip_attributes" do - it { expect(Milestone).to respond_to(:strip_attributes) } + describe ".strip_attributes!" do + it { expect(Milestone).to respond_to(:strip_attributes!) } it { expect(Milestone.strip_attrs).to include(:title) } end - describe "#strip_attributes" do + describe "#strip_attributes!" do before do milestone.title = ' 8.3 ' milestone.valid? diff --git a/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb b/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb new file mode 100644 index 00000000000..0a71699971e --- /dev/null +++ b/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe VulnerabilityFindingSignatureHelpers do + let(:cls) do + Class.new do + include VulnerabilityFindingSignatureHelpers + attr_accessor :algorithm_type + + def initialize(algorithm_type) + @algorithm_type = algorithm_type + end + end + end + + describe '#priority' do + it 'returns numeric values of the priority string' do + expect(cls.new('scope_offset').priority).to eq(3) + expect(cls.new('location').priority).to eq(2) + expect(cls.new('hash').priority).to eq(1) + end + end + + describe '#self.priority' do + it 'returns the numeric value of the provided string' do + expect(cls.priority('scope_offset')).to eq(3) + expect(cls.priority('location')).to eq(2) + expect(cls.priority('hash')).to eq(1) + end + end +end diff --git a/spec/models/concerns/where_composite_spec.rb b/spec/models/concerns/where_composite_spec.rb index fb23e6bfe1d..5e67f2f5b65 100644 --- a/spec/models/concerns/where_composite_spec.rb +++ b/spec/models/concerns/where_composite_spec.rb @@ -8,7 +8,7 @@ RSpec.describe WhereComposite do let(:model) do tbl_name = test_table_name - Class.new(ActiveRecord::Base) do + Class.new(ApplicationRecord) do self.table_name = tbl_name include WhereComposite @@ -16,7 +16,7 @@ RSpec.describe WhereComposite do end def connection - ActiveRecord::Base.connection + ApplicationRecord.connection end before_all do diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index a53db07cc59..846dfb30928 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -323,7 +323,7 @@ RSpec.describe ContainerRepository do context 'with a subgroup' do let_it_be(:test_group) { create(:group) } let_it_be(:another_project) { create(:project, path: 'test', group: test_group) } - let_it_be(:project3) { create(:project, path: 'test3', group: test_group, container_registry_enabled: false) } + let_it_be(:project3) { create(:project, :container_registry_disabled, path: 'test3', group: test_group) } let_it_be(:another_repository) do create(:container_repository, name: 'my_image', project: another_project) diff --git a/spec/models/customer_relations/organization_spec.rb b/spec/models/customer_relations/organization_spec.rb new file mode 100644 index 00000000000..b79b5748156 --- /dev/null +++ b/spec/models/customer_relations/organization_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CustomerRelations::Organization, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:group).with_foreign_key('group_id') } + end + + describe 'validations' do + subject { create(:organization) } + + it { is_expected.to validate_presence_of(:group) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).case_insensitive.scoped_to([:group_id]) } + 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 '#name' do + it 'strips name' do + organization = described_class.new(name: ' GitLab ') + organization.valid? + + expect(organization.name).to eq('GitLab') + end + 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') } + + it 'strips name' do + expect(described_class.find_by_name(group.id, 'TEST')).to eq([organiztion1]) + end + end +end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index c9f7895a616..88451307efb 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -22,6 +22,32 @@ RSpec.describe DeployToken do it { is_expected.to validate_presence_of(:deploy_token_type) } end + shared_examples 'invalid group deploy token' do + context 'revoked' do + before do + deploy_token.update_column(:revoked, true) + end + + it { is_expected.to eq(false) } + end + + context 'expired' do + before do + deploy_token.update!(expires_at: Date.today - 1.month) + end + + it { is_expected.to eq(false) } + end + + context 'project type' do + before do + deploy_token.update_column(:deploy_token_type, 2) + end + + it { is_expected.to eq(false) } + end + end + describe 'deploy_token_type validations' do context 'when a deploy token is associated to a group' do it 'does not allow setting a project to it' do @@ -70,6 +96,50 @@ RSpec.describe DeployToken do end end + describe '#valid_for_dependency_proxy?' do + let_it_be_with_reload(:deploy_token) { create(:deploy_token, :group, :dependency_proxy_scopes) } + + subject { deploy_token.valid_for_dependency_proxy? } + + it { is_expected.to eq(true) } + + it_behaves_like 'invalid group deploy token' + + context 'insufficient scopes' do + before do + deploy_token.update_column(:write_registry, false) + end + + it { is_expected.to eq(false) } + end + end + + describe '#has_access_to_group?' do + let_it_be(:group) { create(:group) } + let_it_be_with_reload(:deploy_token) { create(:deploy_token, :group) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, group: group, deploy_token: deploy_token) } + + let(:test_group) { group } + + subject { deploy_token.has_access_to_group?(test_group) } + + it { is_expected.to eq(true) } + + it_behaves_like 'invalid group deploy token' + + context 'for a sub group' do + let(:test_group) { create(:group, parent: group) } + + it { is_expected.to eq(true) } + end + + context 'for a different group' do + let(:test_group) { create(:group) } + + it { is_expected.to eq(false) } + end + end + describe '#scopes' do context 'with all the scopes' do let_it_be(:deploy_token) { create(:deploy_token, :all_scopes) } diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index 998204626d3..7a57f895b8a 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -128,11 +128,20 @@ RSpec.describe DiffDiscussion do end describe '#cache_key' do + let(:notes_sha) { Digest::SHA1.hexdigest("#{diff_note.post_processed_cache_key}") } + let(:position_sha) { Digest::SHA1.hexdigest(diff_note.position.to_json) } + it 'returns the cache key with the position sha' do - notes_sha = Digest::SHA1.hexdigest("#{diff_note.id}") - position_sha = Digest::SHA1.hexdigest(diff_note.position.to_json) + expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}::#{position_sha}:") + end - expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{diff_note.latest_cached_markdown_version}:#{subject.id}:#{notes_sha}:#{diff_note.updated_at}::#{position_sha}") + context 'when first note of discussion has diff_note_position' do + let!(:diff_note_position) { create(:diff_note_position, note: diff_note) } + let(:positions_sha) { Digest::SHA1.hexdigest(diff_note_position.position.to_json) } + + it 'includes sha of diff_note_positions position' do + expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}::#{position_sha}:#{positions_sha}") + end end end end diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index 2b33de96e04..212619a1c3d 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -53,10 +53,10 @@ RSpec.describe Discussion do end describe '#cache_key' do - let(:notes_sha) { Digest::SHA1.hexdigest("#{first_note.id}:#{second_note.id}:#{third_note.id}") } + let(:notes_sha) { Digest::SHA1.hexdigest("#{first_note.post_processed_cache_key}:#{second_note.post_processed_cache_key}:#{third_note.post_processed_cache_key}") } - it 'returns the cache key with ID and latest updated note updated at' do - expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{third_note.latest_cached_markdown_version}:#{subject.id}:#{notes_sha}:#{third_note.updated_at}:") + it 'returns the cache key' do + expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}:") end context 'when discussion is resolved' do @@ -65,7 +65,7 @@ RSpec.describe Discussion do end it 'returns the cache key with resolved at' do - expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{third_note.latest_cached_markdown_version}:#{subject.id}:#{notes_sha}:#{third_note.updated_at}:#{subject.resolved_at}") + expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}:#{subject.resolved_at}") end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 18a172b72d7..53561586d61 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -14,6 +14,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do subject(:environment) { create(:environment, project: project) } it { is_expected.to be_kind_of(ReactiveCaching) } + it { is_expected.to nullify_if_blank(:external_url) } it { is_expected.to belong_to(:project).required } it { is_expected.to have_many(:deployments) } @@ -214,6 +215,24 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe '.auto_deletable' do + subject { described_class.auto_deletable(limit) } + + let(:limit) { 100 } + + context 'when environment is auto-deletable' do + let!(:environment) { create(:environment, :auto_deletable) } + + it { is_expected.to eq([environment]) } + end + + context 'when environment is not auto-deletable' do + let!(:environment) { create(:environment) } + + it { is_expected.to be_empty } + end + end + describe '.stop_actions' do subject { environments.stop_actions } @@ -412,15 +431,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end - describe '#nullify_external_url' do - it 'replaces a blank url with nil' do - env = build(:environment, external_url: "") - - expect(env.save).to be true - expect(env.external_url).to be_nil - end - end - describe '#includes_commit?' do let(:project) { create(:project, :repository) } diff --git a/spec/models/error_tracking/client_key_spec.rb b/spec/models/error_tracking/client_key_spec.rb new file mode 100644 index 00000000000..54176a32f63 --- /dev/null +++ b/spec/models/error_tracking/client_key_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ErrorTracking::ClientKey, type: :model do + describe 'relationships' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:public_key) } + it { is_expected.to validate_length_of(:public_key).is_at_most(255) } + end + + describe '#generate_key' do + it { expect(subject.public_key).to be_present } + it { expect(subject.public_key).to start_with('glet_') } + end +end diff --git a/spec/models/error_tracking/error_event_spec.rb b/spec/models/error_tracking/error_event_spec.rb index 331661f88cc..8e20eb25353 100644 --- a/spec/models/error_tracking/error_event_spec.rb +++ b/spec/models/error_tracking/error_event_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe ErrorTracking::ErrorEvent, type: :model do + let_it_be(:event) { create(:error_tracking_error_event) } + describe 'relationships' do it { is_expected.to belong_to(:error) } end @@ -11,4 +13,33 @@ RSpec.describe ErrorTracking::ErrorEvent, type: :model do it { is_expected.to validate_presence_of(:description) } it { is_expected.to validate_presence_of(:occurred_at) } end + + describe '#stacktrace' do + it 'generates a correct stacktrace in expected format' do + expected_context = [ + [132, " end\n"], + [133, "\n"], + [134, " begin\n"], + [135, " block.call(work, *extra)\n"], + [136, " rescue Exception => e\n"], + [137, " STDERR.puts \"Error reached top of thread-pool: #\{e.message\} (#\{e.class\})\"\n"], + [138, " end\n"] + ] + + expected_entry = { + 'lineNo' => 135, + 'context' => expected_context, + 'filename' => 'puma/thread_pool.rb', + 'function' => 'block in spawn_thread', + 'colNo' => 0 + } + + expect(event.stacktrace).to be_kind_of(Array) + expect(event.stacktrace.first).to eq(expected_entry) + end + end + + describe '#to_sentry_error_event' do + it { expect(event.to_sentry_error_event).to be_kind_of(Gitlab::ErrorTracking::ErrorEvent) } + end end diff --git a/spec/models/error_tracking/error_spec.rb b/spec/models/error_tracking/error_spec.rb index 8591802d15c..57899985daf 100644 --- a/spec/models/error_tracking/error_spec.rb +++ b/spec/models/error_tracking/error_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe ErrorTracking::Error, type: :model do + let_it_be(:error) { create(:error_tracking_error) } + describe 'relationships' do it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:events) } @@ -13,4 +15,16 @@ RSpec.describe ErrorTracking::Error, type: :model do it { is_expected.to validate_presence_of(:description) } it { is_expected.to validate_presence_of(:actor) } end + + describe '#title' do + it { expect(error.title).to eq('ActionView::MissingTemplate Missing template posts/edit') } + end + + describe '#to_sentry_error' do + it { expect(error.to_sentry_error).to be_kind_of(Gitlab::ErrorTracking::Error) } + end + + describe '#to_sentry_detailed_error' do + it { expect(error.to_sentry_detailed_error).to be_kind_of(Gitlab::ErrorTracking::DetailedError) } + end end 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 3ae0666f7d0..7be61f4950e 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -54,20 +54,22 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do valid_api_url = 'http://example.com/api/0/projects/org-slug/proj-slug/' valid_token = 'token' - where(:enabled, :token, :api_url, :valid?) do - true | nil | nil | false - true | nil | valid_api_url | false - true | valid_token | nil | false - true | valid_token | valid_api_url | true - false | nil | nil | true - false | nil | valid_api_url | true - false | valid_token | nil | true - false | valid_token | valid_api_url | true + where(:enabled, :integrated, :token, :api_url, :valid?) do + true | true | nil | nil | true + true | false | nil | nil | false + true | false | nil | valid_api_url | false + true | false | valid_token | nil | false + true | false | valid_token | valid_api_url | true + false | false | nil | nil | true + false | false | nil | valid_api_url | true + false | false | valid_token | nil | true + false | false | valid_token | valid_api_url | true end with_them do before do subject.enabled = enabled + subject.integrated = integrated subject.token = token subject.api_url = api_url end @@ -241,7 +243,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end describe '#issue_details' do - let(:issue) { build(:detailed_error_tracking_error) } + let(:issue) { build(:error_tracking_sentry_detailed_error) } let(:sentry_client) { double('sentry_client', issue_details: issue) } let(:commit_id) { issue.first_release_version } @@ -472,4 +474,25 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do expect(subject.list_sentry_issues(params)).to eq(nil) end end + + describe '#sentry_enabled' do + using RSpec::Parameterized::TableSyntax + + where(:enabled, :integrated, :feature_flag, :sentry_enabled) do + true | false | false | true + true | true | false | true + true | true | true | false + false | false | false | false + end + + with_them do + before do + subject.enabled = enabled + subject.integrated = integrated + stub_feature_flags(integrated_error_tracking: feature_flag) + end + + it { expect(subject.sentry_enabled).to eq(sentry_enabled) } + end + end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index fc229dcaa22..41510b7aa1c 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -982,9 +982,9 @@ RSpec.describe Event do build(:design_event, trait).action_name end - expect(created).to eq('uploaded') - expect(updated).to eq('revised') - expect(destroyed).to eq('deleted') + expect(created).to eq('added') + expect(updated).to eq('updated') + expect(destroyed).to eq('removed') end it 'handles correct push_action' do diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index 7f0d1e69924..ea5d2b27028 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -258,7 +258,7 @@ RSpec.describe Experiment do let(:variant) { :experimental } it 'does not initiate a transaction' do - expect(ActiveRecord::Base.connection).not_to receive(:transaction) + expect(Experiment.connection).not_to receive(:transaction) subject end @@ -360,7 +360,7 @@ RSpec.describe Experiment do let(:context) { {} } it 'does not initiate a transaction' do - expect(ActiveRecord::Base.connection).not_to receive(:transaction) + expect(Experiment.connection).not_to receive(:transaction) subject end diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index 997d9bbec72..7a1799c670e 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/spec/models/gpg_signature_spec.rb @@ -54,8 +54,10 @@ RSpec.describe GpgSignature do end it 'does not raise an error in case of a race condition' do - expect(described_class).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) - allow(described_class).to receive(:find_or_create_by).and_call_original + expect(described_class).to receive(:find_by).and_return(nil, double(described_class, persisted?: true)) + + expect(described_class).to receive(:create).and_raise(ActiveRecord::RecordNotUnique) + allow(described_class).to receive(:create).and_call_original described_class.safe_create!(attributes) end diff --git a/spec/models/group_deploy_token_spec.rb b/spec/models/group_deploy_token_spec.rb index d38abafa7ed..bc44c473ddb 100644 --- a/spec/models/group_deploy_token_spec.rb +++ b/spec/models/group_deploy_token_spec.rb @@ -3,15 +3,40 @@ require 'spec_helper' RSpec.describe GroupDeployToken, type: :model do - let(:group) { create(:group) } - let(:deploy_token) { create(:deploy_token) } + let_it_be(:group) { create(:group) } + let_it_be(:deploy_token) { create(:deploy_token) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, group: group, deploy_token: deploy_token) } - subject(:group_deploy_token) { create(:group_deploy_token, group: group, deploy_token: deploy_token) } + describe 'relationships' do + it { is_expected.to belong_to :group } + it { is_expected.to belong_to :deploy_token } + end - it { is_expected.to belong_to :group } - it { is_expected.to belong_to :deploy_token } + describe 'validation' do + it { is_expected.to validate_presence_of :deploy_token } + it { is_expected.to validate_presence_of :group } + it { is_expected.to validate_uniqueness_of(:deploy_token_id).scoped_to(:group_id) } + end - it { is_expected.to validate_presence_of :deploy_token } - it { is_expected.to validate_presence_of :group } - it { is_expected.to validate_uniqueness_of(:deploy_token_id).scoped_to(:group_id) } + describe '#has_access_to_group?' do + subject { group_deploy_token.has_access_to_group?(test_group) } + + context 'for itself' do + let(:test_group) { group } + + it { is_expected.to eq(true) } + end + + context 'for a subgroup' do + let(:test_group) { create(:group, parent: group) } + + it { is_expected.to eq(true) } + end + + context 'for other group' do + let(:test_group) { create(:group) } + + it { is_expected.to eq(false) } + end + end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0a08b15a1eb..ddf12c8e4c4 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -640,6 +640,12 @@ RSpec.describe Group do it { is_expected.to match_array([private_group, internal_group]) } end + describe 'private_only' do + subject { described_class.private_only.to_a } + + it { is_expected.to match_array([private_group]) } + end + describe 'with_onboarding_progress' do subject { described_class.with_onboarding_progress } @@ -2598,6 +2604,21 @@ RSpec.describe Group do it { is_expected.to eq(Set.new([child_1.id])) } end + describe '.timelogs' do + let(:project) { create(:project, namespace: group) } + let(:issue) { create(:issue, project: project) } + let(:other_project) { create(:project, namespace: create(:group)) } + let(:other_issue) { create(:issue, project: other_project) } + + let!(:timelog1) { create(:timelog, issue: issue) } + let!(:timelog2) { create(:timelog, issue: other_issue) } + let!(:timelog3) { create(:timelog, issue: issue) } + + it 'returns timelogs belonging to the group' do + expect(group.timelogs).to contain_exactly(timelog1, timelog3) + end + end + describe '#to_ability_name' do it 'returns group' do group = build(:group) diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index a99263078b3..17cb5da977a 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -16,6 +16,21 @@ RSpec.describe SystemHook do end end + describe 'validations' do + describe 'url' do + let(:url) { 'http://localhost:9000' } + + it { is_expected.not_to allow_value(url).for(:url) } + + it 'is valid if application settings allow local requests from system hooks' do + settings = ApplicationSetting.new(allow_local_requests_from_system_hooks: true) + allow(ApplicationSetting).to receive(:current).and_return(settings) + + is_expected.to allow_value(url).for(:url) + end + end + end + describe "execute", :sidekiq_might_not_need_inline do let(:system_hook) { create(:system_hook) } let(:user) { create(:user) } diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 1761b537dc0..c68ad3bf0c4 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -32,6 +32,19 @@ RSpec.describe WebHook do it { is_expected.not_to allow_value('ftp://example.com').for(:url) } it { is_expected.not_to allow_value('herp-and-derp').for(:url) } + context 'when url is local' do + let(:url) { 'http://localhost:9000' } + + it { is_expected.not_to allow_value(url).for(:url) } + + it 'is valid if application settings allow local requests from web hooks' do + settings = ApplicationSetting.new(allow_local_requests_from_web_hooks_and_services: true) + allow(ApplicationSetting).to receive(:current).and_return(settings) + + is_expected.to allow_value(url).for(:url) + end + end + it 'strips :url before saving it' do hook.url = ' https://example.com ' hook.save! @@ -267,6 +280,15 @@ RSpec.describe WebHook do end end + shared_examples 'is tolerant of invalid records' do + specify do + hook.url = nil + + expect(hook).to be_invalid + run_expectation + end + end + describe '#enable!' do it 'makes a hook executable if it was marked as failed' do hook.recent_failures = 1000 @@ -281,15 +303,17 @@ RSpec.describe WebHook do end it 'does not update hooks unless necessary' do - expect(hook).not_to receive(:update!) + sql_count = ActiveRecord::QueryRecorder.new { hook.enable! }.count - hook.enable! + expect(sql_count).to eq(0) end - it 'is idempotent on executable hooks' do - expect(hook).not_to receive(:update!) + include_examples 'is tolerant of invalid records' do + def run_expectation + hook.recent_failures = 1000 - expect { hook.enable! }.not_to change(hook, :executable?) + expect { hook.enable! }.to change(hook, :executable?).from(false).to(true) + end end end @@ -307,6 +331,12 @@ RSpec.describe WebHook do expect { hook.backoff! }.not_to change(hook, :backoff_count) end + + include_examples 'is tolerant of invalid records' do + def run_expectation + expect { hook.backoff! }.to change(hook, :backoff_count).by(1) + end + end end describe 'failed!' do @@ -314,11 +344,18 @@ RSpec.describe WebHook do expect { hook.failed! }.to change(hook, :recent_failures).by(1) end - it 'does not allow the failure count to exceed the maximum value' do + it 'does not update the hook if the the failure count exceeds the maximum value' do hook.recent_failures = described_class::MAX_FAILURES - expect(hook).not_to receive(:update!) - expect { hook.failed! }.not_to change(hook, :recent_failures) + sql_count = ActiveRecord::QueryRecorder.new { hook.failed! }.count + + expect(sql_count).to eq(0) + end + + include_examples 'is tolerant of invalid records' do + def run_expectation + expect { hook.failed! }.to change(hook, :recent_failures).by(1) + end end end @@ -326,5 +363,11 @@ RSpec.describe WebHook do it 'disables a hook' do expect { hook.disable! }.to change(hook, :executable?).from(true).to(false) end + + include_examples 'is tolerant of invalid records' do + def run_expectation + expect { hook.disable! }.to change(hook, :executable?).from(true).to(false) + end + 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 new file mode 100644 index 00000000000..f3e7b90cf3c --- /dev/null +++ b/spec/models/incident_management/issuable_escalation_status_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe IncidentManagement::IssuableEscalationStatus do + let_it_be(:issue) { create(:issue) } + + subject(:escalation_status) { build(:incident_management_issuable_escalation_status, issue: issue) } + + it { is_expected.to be_valid } + + describe 'associations' do + it { is_expected.to belong_to(:issue) } + end + + describe 'validatons' do + it { is_expected.to validate_presence_of(:issue) } + it { is_expected.to validate_uniqueness_of(:issue) } + end + + it_behaves_like 'a model including Escalatable' +end diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index d3566ed04c2..9544f0fe6ec 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -96,6 +96,95 @@ RSpec.describe InstanceConfiguration do expect(gitlab_ci[:artifacts_max_size][:value]).to eq(200.megabytes) end end + + describe '#package_file_size_limits' do + let_it_be(:plan1) { create(:plan, name: 'plan1', title: 'Plan 1') } + let_it_be(:plan2) { create(:plan, name: 'plan2', title: 'Plan 2') } + + before do + create(:plan_limits, + plan: plan1, + conan_max_file_size: 1001, + maven_max_file_size: 1002, + npm_max_file_size: 1003, + nuget_max_file_size: 1004, + pypi_max_file_size: 1005, + terraform_module_max_file_size: 1006, + generic_packages_max_file_size: 1007 + ) + create(:plan_limits, + plan: plan2, + conan_max_file_size: 1101, + maven_max_file_size: 1102, + npm_max_file_size: 1103, + nuget_max_file_size: 1104, + pypi_max_file_size: 1105, + terraform_module_max_file_size: 1106, + generic_packages_max_file_size: 1107 + ) + end + + it 'returns package file size limits' do + file_size_limits = subject.settings[:package_file_size_limits] + + expect(file_size_limits[:Plan1]).to eq({ conan: 1001, maven: 1002, npm: 1003, nuget: 1004, pypi: 1005, terraform_module: 1006, generic: 1007 }) + expect(file_size_limits[:Plan2]).to eq({ conan: 1101, maven: 1102, npm: 1103, nuget: 1104, pypi: 1105, terraform_module: 1106, generic: 1107 }) + end + end + + describe '#rate_limits' do + before do + Gitlab::CurrentSettings.current_application_settings.update!( + throttle_unauthenticated_enabled: false, + throttle_unauthenticated_requests_per_period: 1001, + throttle_unauthenticated_period_in_seconds: 1002, + throttle_authenticated_api_enabled: true, + throttle_authenticated_api_requests_per_period: 1003, + throttle_authenticated_api_period_in_seconds: 1004, + throttle_authenticated_web_enabled: true, + throttle_authenticated_web_requests_per_period: 1005, + throttle_authenticated_web_period_in_seconds: 1006, + throttle_protected_paths_enabled: true, + throttle_protected_paths_requests_per_period: 1007, + throttle_protected_paths_period_in_seconds: 1008, + throttle_unauthenticated_packages_api_enabled: false, + throttle_unauthenticated_packages_api_requests_per_period: 1009, + throttle_unauthenticated_packages_api_period_in_seconds: 1010, + throttle_authenticated_packages_api_enabled: true, + throttle_authenticated_packages_api_requests_per_period: 1011, + throttle_authenticated_packages_api_period_in_seconds: 1012, + issues_create_limit: 1013, + notes_create_limit: 1014, + project_export_limit: 1015, + project_download_export_limit: 1016, + project_import_limit: 1017, + group_export_limit: 1018, + group_download_export_limit: 1019, + group_import_limit: 1020, + raw_blob_request_limit: 1021 + ) + end + + it 'returns rate limits from application settings' do + rate_limits = subject.settings[:rate_limits] + + expect(rate_limits[:unauthenticated]).to eq({ enabled: false, requests_per_period: 1001, period_in_seconds: 1002 }) + expect(rate_limits[:authenticated_api]).to eq({ enabled: true, requests_per_period: 1003, period_in_seconds: 1004 }) + expect(rate_limits[:authenticated_web]).to eq({ enabled: true, requests_per_period: 1005, period_in_seconds: 1006 }) + expect(rate_limits[:protected_paths]).to eq({ enabled: true, requests_per_period: 1007, period_in_seconds: 1008 }) + expect(rate_limits[:unauthenticated_packages_api]).to eq({ enabled: false, requests_per_period: 1009, period_in_seconds: 1010 }) + expect(rate_limits[:authenticated_packages_api]).to eq({ enabled: true, requests_per_period: 1011, period_in_seconds: 1012 }) + expect(rate_limits[:issue_creation]).to eq({ enabled: true, requests_per_period: 1013, period_in_seconds: 60 }) + expect(rate_limits[:note_creation]).to eq({ enabled: true, requests_per_period: 1014, period_in_seconds: 60 }) + expect(rate_limits[:project_export]).to eq({ enabled: true, requests_per_period: 1015, period_in_seconds: 60 }) + expect(rate_limits[:project_export_download]).to eq({ enabled: true, requests_per_period: 1016, period_in_seconds: 60 }) + expect(rate_limits[:project_import]).to eq({ enabled: true, requests_per_period: 1017, period_in_seconds: 60 }) + expect(rate_limits[:group_export]).to eq({ enabled: true, requests_per_period: 1018, period_in_seconds: 60 }) + 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 }) + end + end end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index ab4027170b2..f5f6a425fdd 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -21,38 +21,31 @@ RSpec.describe Integration do it { is_expected.to validate_presence_of(:type) } it { is_expected.to validate_exclusion_of(:type).in_array(described_class::BASE_CLASSES) } - where(:project_id, :group_id, :template, :instance, :valid) do - 1 | nil | false | false | true - nil | 1 | false | false | true - nil | nil | true | false | true - nil | nil | false | true | true - nil | nil | false | false | false - nil | nil | true | true | false - 1 | 1 | false | false | false - 1 | nil | true | false | false - 1 | nil | false | true | false - nil | 1 | true | false | false - nil | 1 | false | true | false + where(:project_id, :group_id, :instance, :valid) do + 1 | nil | false | true + nil | 1 | false | true + nil | nil | true | true + nil | nil | false | false + 1 | 1 | false | false + 1 | nil | false | true + 1 | nil | true | false + nil | 1 | false | true + nil | 1 | true | false end with_them do it 'validates the service' do - expect(build(:service, project_id: project_id, group_id: group_id, template: template, instance: instance).valid?).to eq(valid) + expect(build(:service, project_id: project_id, group_id: group_id, instance: instance).valid?).to eq(valid) end end context 'with existing services' do before_all do - create(:service, :template) create(:service, :instance) create(:service, project: project) create(:service, group: group, project: nil) end - it 'allows only one service template per type' do - expect(build(:service, :template)).to be_invalid - end - it 'allows only one instance service per type' do expect(build(:service, :instance)).to be_invalid end @@ -68,6 +61,24 @@ RSpec.describe Integration do end describe 'Scopes' do + describe '.with_default_settings' do + it 'returns the correct integrations' do + instance_integration = create(:integration, :instance) + inheriting_integration = create(:integration, inherit_from_id: instance_integration.id) + + expect(described_class.with_default_settings).to match_array([inheriting_integration]) + end + end + + describe '.with_custom_settings' do + it 'returns the correct integrations' do + instance_integration = create(:integration, :instance) + create(:integration, inherit_from_id: instance_integration.id) + + expect(described_class.with_custom_settings).to match_array([instance_integration]) + end + end + describe '.by_type' do let!(:service1) { create(:jira_integration) } let!(:service2) { create(:jira_integration) } @@ -263,192 +274,108 @@ RSpec.describe Integration do end end - describe 'template' do - shared_examples 'retrieves service templates' do - it 'returns the available service templates' do - expect(Integration.find_or_create_templates.pluck(:type)).to match_array(Integration.available_integration_types(include_project_specific: false)) + describe '.build_from_integration' do + context 'when integration is invalid' do + let(:invalid_integration) do + build(:prometheus_integration, :template, active: true, properties: {}) + .tap { |integration| integration.save!(validate: false) } end - end - describe '.find_or_create_templates' do - it 'creates service templates' do - total = Integration.available_integration_names(include_project_specific: false).size + it 'sets integration to inactive' do + integration = described_class.build_from_integration(invalid_integration, project_id: project.id) - expect { Integration.find_or_create_templates }.to change(Integration, :count).from(0).to(total) + expect(integration).to be_valid + expect(integration.active).to be false end + end - it_behaves_like 'retrieves service templates' - - context 'with all existing templates' do - before do - Integration.insert_all( - Integration.available_integration_types(include_project_specific: false).map { |type| { template: true, type: type } } - ) - end - - it 'does not create service templates' do - expect { Integration.find_or_create_templates }.not_to change { Integration.count } - end + context 'when integration is an instance-level integration' do + let(:instance_integration) { create(:jira_integration, :instance) } - it_behaves_like 'retrieves service templates' + it 'sets inherit_from_id from integration' do + integration = described_class.build_from_integration(instance_integration, project_id: project.id) - context 'with a previous existing service (Previous) and a new service (Asana)' do - before do - Integration.insert({ type: 'PreviousService', template: true }) - Integration.delete_by(type: 'AsanaService', template: true) - end - - it_behaves_like 'retrieves service templates' - end + expect(integration.inherit_from_id).to eq(instance_integration.id) end + end - context 'with a few existing templates' do - before do - create(:jira_integration, :template) - end - - it 'creates the rest of the service templates' do - total = Integration.available_integration_names(include_project_specific: false).size + context 'when integration is a group-level integration' do + let(:group_integration) { create(:jira_integration, group: group, project: nil) } - expect { Integration.find_or_create_templates }.to change(Integration, :count).from(1).to(total) - end + it 'sets inherit_from_id from integration' do + integration = described_class.build_from_integration(group_integration, project_id: project.id) - it_behaves_like 'retrieves service templates' + expect(integration.inherit_from_id).to eq(group_integration.id) end end - describe '.build_from_integration' do - context 'when integration is invalid' do - let(:template_integration) do - build(:prometheus_integration, :template, active: true, properties: {}) - .tap { |integration| integration.save!(validate: false) } - end - - it 'sets integration to inactive' do - integration = described_class.build_from_integration(template_integration, project_id: project.id) - - expect(integration).to be_valid - expect(integration.active).to be false - end + describe 'build issue tracker from an integration' do + let(:url) { 'http://jira.example.com' } + let(:api_url) { 'http://api-jira.example.com' } + let(:username) { 'jira-username' } + let(:password) { 'jira-password' } + let(:data_params) do + { + url: url, api_url: api_url, + username: username, password: password + } end - context 'when integration is an instance-level integration' do - let(:instance_integration) { create(:jira_integration, :instance) } - - it 'sets inherit_from_id from integration' do - integration = described_class.build_from_integration(instance_integration, project_id: project.id) + shared_examples 'service creation from an integration' do + it 'creates a correct service for a project integration' do + service = described_class.build_from_integration(integration, project_id: project.id) - expect(integration.inherit_from_id).to eq(instance_integration.id) + expect(service).to be_active + expect(service.url).to eq(url) + expect(service.api_url).to eq(api_url) + expect(service.username).to eq(username) + expect(service.password).to eq(password) + expect(service.instance).to eq(false) + expect(service.project).to eq(project) + expect(service.group).to eq(nil) end - end - - context 'when integration is a group-level integration' do - let(:group_integration) { create(:jira_integration, group: group, project: nil) } - - it 'sets inherit_from_id from integration' do - integration = described_class.build_from_integration(group_integration, project_id: project.id) - expect(integration.inherit_from_id).to eq(group_integration.id) + it 'creates a correct service for a group integration' do + service = described_class.build_from_integration(integration, group_id: group.id) + + expect(service).to be_active + expect(service.url).to eq(url) + expect(service.api_url).to eq(api_url) + expect(service.username).to eq(username) + expect(service.password).to eq(password) + expect(service.instance).to eq(false) + expect(service.project).to eq(nil) + expect(service.group).to eq(group) end end - describe 'build issue tracker from an integration' do - let(:url) { 'http://jira.example.com' } - let(:api_url) { 'http://api-jira.example.com' } - let(:username) { 'jira-username' } - let(:password) { 'jira-password' } - let(:data_params) do - { - url: url, api_url: api_url, - username: username, password: password - } - end - - shared_examples 'service creation from an integration' do - it 'creates a correct service for a project integration' do - service = described_class.build_from_integration(integration, project_id: project.id) - - expect(service).to be_active - expect(service.url).to eq(url) - expect(service.api_url).to eq(api_url) - expect(service.username).to eq(username) - expect(service.password).to eq(password) - expect(service.template).to eq(false) - expect(service.instance).to eq(false) - expect(service.project).to eq(project) - expect(service.group).to eq(nil) - end - - it 'creates a correct service for a group integration' do - service = described_class.build_from_integration(integration, group_id: group.id) - - expect(service).to be_active - expect(service.url).to eq(url) - expect(service.api_url).to eq(api_url) - expect(service.username).to eq(username) - expect(service.password).to eq(password) - expect(service.template).to eq(false) - expect(service.instance).to eq(false) - expect(service.project).to eq(nil) - expect(service.group).to eq(group) - end + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 + context 'when data is stored in properties' do + let(:properties) { data_params } + let!(:integration) do + create(:jira_integration, :without_properties_callback, properties: properties.merge(additional: 'something')) end - # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 - context 'when data are stored in properties' do - let(:properties) { data_params } - let!(:integration) do - create(:jira_integration, :without_properties_callback, template: true, properties: properties.merge(additional: 'something')) - end + it_behaves_like 'service creation from an integration' + end - it_behaves_like 'service creation from an integration' + context 'when data are stored in separated fields' do + let(:integration) do + create(:jira_integration, data_params.merge(properties: {})) end - context 'when data are stored in separated fields' do - let(:integration) do - create(:jira_integration, :template, data_params.merge(properties: {})) - end - - it_behaves_like 'service creation from an integration' - end + it_behaves_like 'service creation from an integration' + end - context 'when data are stored in both properties and separated fields' do - let(:properties) { data_params } - let(:integration) do - create(:jira_integration, :without_properties_callback, active: true, template: true, properties: properties).tap do |integration| - create(:jira_tracker_data, data_params.merge(integration: integration)) - end + context 'when data are stored in both properties and separated fields' do + let(:properties) { data_params } + let(:integration) do + create(:jira_integration, :without_properties_callback, active: true, properties: properties).tap do |integration| + create(:jira_tracker_data, data_params.merge(integration: integration)) end - - it_behaves_like 'service creation from an integration' end - end - end - describe "for pushover service" do - let!(:service_template) do - Integrations::Pushover.create!( - template: true, - properties: { - device: 'MyDevice', - sound: 'mic', - priority: 4, - api_key: '123456789' - }) - end - - describe 'is prefilled for projects pushover service' do - it "has all fields prefilled" do - integration = project.find_or_initialize_integration('pushover') - - expect(integration).to have_attributes( - template: eq(false), - device: eq('MyDevice'), - sound: eq('mic'), - priority: eq(4), - api_key: eq('123456789') - ) - end + it_behaves_like 'service creation from an integration' end end end @@ -510,121 +437,109 @@ RSpec.describe Integration do end describe '.create_from_active_default_integrations' do - context 'with an active integration template' do - let_it_be(:template_integration) { create(:prometheus_integration, :template, api_url: 'https://prometheus.template.com/') } + context 'with an active instance-level integration' do + let!(:instance_integration) { create(:prometheus_integration, :instance, api_url: 'https://prometheus.instance.com/') } - it 'creates an integration from the template' do - described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + it 'creates an integration from the instance-level integration' do + described_class.create_from_active_default_integrations(project, :project_id) expect(project.reload.integrations.size).to eq(1) - expect(project.reload.integrations.first.api_url).to eq(template_integration.api_url) - expect(project.reload.integrations.first.inherit_from_id).to be_nil + expect(project.reload.integrations.first.api_url).to eq(instance_integration.api_url) + expect(project.reload.integrations.first.inherit_from_id).to eq(instance_integration.id) end - context 'with an active instance-level integration' do - let!(:instance_integration) { create(:prometheus_integration, :instance, api_url: 'https://prometheus.instance.com/') } - + context 'passing a group' do it 'creates an integration from the instance-level integration' do - described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + described_class.create_from_active_default_integrations(group, :group_id) + + expect(group.reload.integrations.size).to eq(1) + expect(group.reload.integrations.first.api_url).to eq(instance_integration.api_url) + expect(group.reload.integrations.first.inherit_from_id).to eq(instance_integration.id) + end + end + + context 'with an active group-level integration' do + let!(:group_integration) { create(:prometheus_integration, group: group, project: nil, api_url: 'https://prometheus.group.com/') } + + it 'creates an integration from the group-level integration' do + described_class.create_from_active_default_integrations(project, :project_id) expect(project.reload.integrations.size).to eq(1) - expect(project.reload.integrations.first.api_url).to eq(instance_integration.api_url) - expect(project.reload.integrations.first.inherit_from_id).to eq(instance_integration.id) + expect(project.reload.integrations.first.api_url).to eq(group_integration.api_url) + expect(project.reload.integrations.first.inherit_from_id).to eq(group_integration.id) end context 'passing a group' do - it 'creates an integration from the instance-level integration' do - described_class.create_from_active_default_integrations(group, :group_id) + let!(:subgroup) { create(:group, parent: group) } - expect(group.reload.integrations.size).to eq(1) - expect(group.reload.integrations.first.api_url).to eq(instance_integration.api_url) - expect(group.reload.integrations.first.inherit_from_id).to eq(instance_integration.id) + it 'creates an integration from the group-level integration' do + described_class.create_from_active_default_integrations(subgroup, :group_id) + + expect(subgroup.reload.integrations.size).to eq(1) + expect(subgroup.reload.integrations.first.api_url).to eq(group_integration.api_url) + expect(subgroup.reload.integrations.first.inherit_from_id).to eq(group_integration.id) end end - context 'with an active group-level integration' do - let!(:group_integration) { create(:prometheus_integration, group: group, project: nil, api_url: 'https://prometheus.group.com/') } + context 'with an active subgroup' do + let!(:subgroup_integration) { create(:prometheus_integration, group: subgroup, project: nil, api_url: 'https://prometheus.subgroup.com/') } + let!(:subgroup) { create(:group, parent: group) } + let(:project) { create(:project, group: subgroup) } - it 'creates an integration from the group-level integration' do - described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + it 'creates an integration from the subgroup-level integration' do + described_class.create_from_active_default_integrations(project, :project_id) expect(project.reload.integrations.size).to eq(1) - expect(project.reload.integrations.first.api_url).to eq(group_integration.api_url) - expect(project.reload.integrations.first.inherit_from_id).to eq(group_integration.id) + expect(project.reload.integrations.first.api_url).to eq(subgroup_integration.api_url) + expect(project.reload.integrations.first.inherit_from_id).to eq(subgroup_integration.id) end context 'passing a group' do - let!(:subgroup) { create(:group, parent: group) } - - it 'creates an integration from the group-level integration' do - described_class.create_from_active_default_integrations(subgroup, :group_id) - - expect(subgroup.reload.integrations.size).to eq(1) - expect(subgroup.reload.integrations.first.api_url).to eq(group_integration.api_url) - expect(subgroup.reload.integrations.first.inherit_from_id).to eq(group_integration.id) - end - end + let!(:sub_subgroup) { create(:group, parent: subgroup) } - context 'with an active subgroup' do - let!(:subgroup_integration) { create(:prometheus_integration, group: subgroup, project: nil, api_url: 'https://prometheus.subgroup.com/') } - let!(:subgroup) { create(:group, parent: group) } - let(:project) { create(:project, group: subgroup) } + context 'traversal queries' do + shared_examples 'correct ancestor order' do + it 'creates an integration from the subgroup-level integration' do + described_class.create_from_active_default_integrations(sub_subgroup, :group_id) - it 'creates an integration from the subgroup-level integration' do - described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + sub_subgroup.reload - expect(project.reload.integrations.size).to eq(1) - expect(project.reload.integrations.first.api_url).to eq(subgroup_integration.api_url) - expect(project.reload.integrations.first.inherit_from_id).to eq(subgroup_integration.id) - end + expect(sub_subgroup.integrations.size).to eq(1) + expect(sub_subgroup.integrations.first.api_url).to eq(subgroup_integration.api_url) + expect(sub_subgroup.integrations.first.inherit_from_id).to eq(subgroup_integration.id) + end - context 'passing a group' do - let!(:sub_subgroup) { create(:group, parent: subgroup) } + context 'having an integration inheriting settings' do + let!(:subgroup_integration) { create(:prometheus_integration, group: subgroup, project: nil, inherit_from_id: group_integration.id, api_url: 'https://prometheus.subgroup.com/') } - context 'traversal queries' do - shared_examples 'correct ancestor order' do - it 'creates an integration from the subgroup-level integration' do + it 'creates an integration from the group-level integration' do described_class.create_from_active_default_integrations(sub_subgroup, :group_id) sub_subgroup.reload expect(sub_subgroup.integrations.size).to eq(1) - expect(sub_subgroup.integrations.first.api_url).to eq(subgroup_integration.api_url) - expect(sub_subgroup.integrations.first.inherit_from_id).to eq(subgroup_integration.id) - end - - context 'having an integration inheriting settings' do - let!(:subgroup_integration) { create(:prometheus_integration, group: subgroup, project: nil, inherit_from_id: group_integration.id, api_url: 'https://prometheus.subgroup.com/') } - - it 'creates an integration from the group-level integration' do - described_class.create_from_active_default_integrations(sub_subgroup, :group_id) - - sub_subgroup.reload - - expect(sub_subgroup.integrations.size).to eq(1) - expect(sub_subgroup.integrations.first.api_url).to eq(group_integration.api_url) - expect(sub_subgroup.integrations.first.inherit_from_id).to eq(group_integration.id) - end + expect(sub_subgroup.integrations.first.api_url).to eq(group_integration.api_url) + expect(sub_subgroup.integrations.first.inherit_from_id).to eq(group_integration.id) end end + end - context 'recursive' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - include_examples 'correct ancestor order' + context 'recursive' do + before do + stub_feature_flags(use_traversal_ids: false) end - context 'linear' do - before do - stub_feature_flags(use_traversal_ids: true) + include_examples 'correct ancestor order' + end - sub_subgroup.reload # make sure traversal_ids are reloaded - end + context 'linear' do + before do + stub_feature_flags(use_traversal_ids: true) - include_examples 'correct ancestor order' + sub_subgroup.reload # make sure traversal_ids are reloaded end + + include_examples 'correct ancestor order' end end end diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index 73ebf404828..60ff6685c3d 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do subject(:integration) do described_class.create!( + active: true, project: project, properties: { bamboo_url: bamboo_url, @@ -74,27 +75,27 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do end describe 'Callbacks' do - describe 'before_update :reset_password' do + describe 'before_validation :reset_password' do context 'when a password was previously set' do it 'resets password if url changed' do integration.bamboo_url = 'http://gitlab1.com' - integration.save! + expect(integration).not_to be_valid expect(integration.password).to be_nil end it 'does not reset password if username changed' do integration.username = 'some_name' - integration.save! + expect(integration).to be_valid expect(integration.password).to eq('password') end it "does not reset password if new url is set together with password, even if it's the same password" do integration.bamboo_url = 'http://gitlab_edited.com' integration.password = 'password' - integration.save! + expect(integration).to be_valid expect(integration.password).to eq('password') expect(integration.bamboo_url).to eq('http://gitlab_edited.com') end @@ -107,8 +108,10 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do integration.password = 'password' integration.save! - expect(integration.password).to eq('password') - expect(integration.bamboo_url).to eq('http://gitlab_edited.com') + expect(integration.reload).to have_attributes( + bamboo_url: 'http://gitlab_edited.com', + password: 'password' + ) end end end diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index e2749ab1bc1..677bd4c5e48 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -6,7 +6,8 @@ require 'spec_helper' RSpec.describe Integrations::Datadog do let_it_be(:project) { create(:project) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } - let_it_be(:build) { create(:ci_build, project: project) } + let_it_be(:build) { create(:ci_build, pipeline: pipeline) } + let_it_be(:retried_build) { create(:ci_build, :retried, pipeline: pipeline) } let(:active) { true } let(:dd_site) { 'datadoghq.com' } @@ -139,26 +140,38 @@ RSpec.describe Integrations::Datadog do end describe '#test' do - context 'when request is succesful' do - subject { saved_instance.test(pipeline_data) } + subject(:result) { saved_instance.test(pipeline_data) } - before do - stub_request(:post, expected_hook_url).to_return(body: 'OK') - end + let(:body) { 'OK' } + let(:status) { 200 } + + before do + stub_request(:post, expected_hook_url).to_return(body: body, status: status) + end + + context 'when request is successful with a HTTP 200 status' do it { is_expected.to eq({ success: true, result: 'OK' }) } end - context 'when request fails' do - subject { saved_instance.test(pipeline_data) } + context 'when request is successful with a HTTP 202 status' do + let(:status) { 202 } + + it { is_expected.to eq({ success: true, result: 'OK' }) } + end + + context 'when request fails with a HTTP 500 status' do + let(:status) { 500 } + let(:body) { 'CRASH!!!' } - before do - stub_request(:post, expected_hook_url).to_return(body: 'CRASH!!!', status: 500) - end it { is_expected.to eq({ success: false, result: 'CRASH!!!' }) } end end describe '#execute' do + around do |example| + freeze_time { example.run } + end + before do stub_request(:post, expected_hook_url) saved_instance.execute(data) @@ -166,20 +179,18 @@ RSpec.describe Integrations::Datadog do context 'with pipeline data' do let(:data) { pipeline_data } - let(:expected_headers) do - { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' } - end + let(:expected_headers) { { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' } } + let(:expected_body) { data.with_retried_builds.to_json } - it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers)).to have_been_made } + it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } end context 'with job data' do let(:data) { build_data } - let(:expected_headers) do - { WebHookService::GITLAB_EVENT_HEADER => 'Job Hook' } - end + let(:expected_headers) { { WebHookService::GITLAB_EVENT_HEADER => 'Job Hook' } } + let(:expected_body) { data.to_json } - it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers)).to have_been_made } + it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } end end end diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 9eb2a7fc098..9286d026290 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -200,21 +200,21 @@ RSpec.describe Integrations::Jenkins do it 'resets password if url changed' do jenkins_integration.jenkins_url = 'http://jenkins-edited.example.com/' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to be_nil end it 'resets password if username is blank' do jenkins_integration.username = '' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to be_nil end it 'does not reset password if username changed' do jenkins_integration.username = 'some_name' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to eq('password') end @@ -222,7 +222,7 @@ RSpec.describe Integrations::Jenkins do it 'does not reset password if new url is set together with password, even if it\'s the same password' do jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' jenkins_integration.password = 'password' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to eq('password') expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/') @@ -231,7 +231,7 @@ RSpec.describe Integrations::Jenkins do it 'resets password if url changed, even if setter called multiple times' do jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to be_nil end @@ -253,8 +253,10 @@ RSpec.describe Integrations::Jenkins do jenkins_integration.password = 'password' jenkins_integration.save! - expect(jenkins_integration.password).to eq('password') - expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/') + expect(jenkins_integration.reload).to have_attributes( + jenkins_url: 'http://jenkins_edited.example.com/', + password: 'password' + ) end end end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 6ca72d68bbb..0321b151633 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -280,7 +280,7 @@ RSpec.describe Integrations::Jira do expect(integration.jira_tracker_data.deployment_server?).to be_truthy - integration.update!(api_url: 'http://another.url') + integration.update!(api_url: 'http://another.url', password: password) integration.jira_tracker_data.reload expect(integration.jira_tracker_data.deployment_cloud?).to be_truthy @@ -301,13 +301,13 @@ RSpec.describe Integrations::Jira do end it 'calls serverInfo for url' do - integration.update!(url: 'http://first.url') + integration.update!(url: 'http://first.url', password: password) expect(WebMock).to have_requested(:get, /serverInfo/) end it 'calls serverInfo for api_url' do - integration.update!(api_url: 'http://another.url') + integration.update!(api_url: 'http://another.url', password: password) expect(WebMock).to have_requested(:get, /serverInfo/) end @@ -334,16 +334,6 @@ RSpec.describe Integrations::Jira do end end - context 'when not allowed to test an instance or group' do - it 'does not update deployment type' do - allow(integration).to receive(:testable?).and_return(false) - - integration.update!(url: 'http://first.url') - - expect(WebMock).not_to have_requested(:get, /serverInfo/) - end - end - context 'stored password invalidation' do context 'when a password was previously set' do context 'when only web url present' do @@ -358,33 +348,33 @@ RSpec.describe Integrations::Jira do it 'resets password if url changed' do integration integration.url = 'http://jira_edited.example.com' - integration.save! - expect(integration.reload.url).to eq('http://jira_edited.example.com') + expect(integration).not_to be_valid + expect(integration.url).to eq('http://jira_edited.example.com') expect(integration.password).to be_nil end it 'does not reset password if url "changed" to the same url as before' do integration.url = 'http://jira.example.com' - integration.save! - expect(integration.reload.url).to eq('http://jira.example.com') + expect(integration).to be_valid + expect(integration.url).to eq('http://jira.example.com') expect(integration.password).not_to be_nil end it 'resets password if url not changed but api url added' do integration.api_url = 'http://jira_edited.example.com/rest/api/2' - integration.save! - expect(integration.reload.api_url).to eq('http://jira_edited.example.com/rest/api/2') + expect(integration).not_to be_valid + expect(integration.api_url).to eq('http://jira_edited.example.com/rest/api/2') expect(integration.password).to be_nil end it 'does not reset password if new url is set together with password, even if it\'s the same password' do integration.url = 'http://jira_edited.example.com' integration.password = password - integration.save! + expect(integration).to be_valid expect(integration.password).to eq(password) expect(integration.url).to eq('http://jira_edited.example.com') end @@ -392,32 +382,32 @@ RSpec.describe Integrations::Jira do it 'resets password if url changed, even if setter called multiple times' do integration.url = 'http://jira1.example.com/rest/api/2' integration.url = 'http://jira1.example.com/rest/api/2' - integration.save! + expect(integration).not_to be_valid expect(integration.password).to be_nil end it 'does not reset password if username changed' do integration.username = 'some_name' - integration.save! - expect(integration.reload.password).to eq(password) + expect(integration).to be_valid + expect(integration.password).to eq(password) end it 'does not reset password if password changed' do integration.url = 'http://jira_edited.example.com' integration.password = 'new_password' - integration.save! - expect(integration.reload.password).to eq('new_password') + expect(integration).to be_valid + expect(integration.password).to eq('new_password') end it 'does not reset password if the password is touched and same as before' do integration.url = 'http://jira_edited.example.com' integration.password = password - integration.save! - expect(integration.reload.password).to eq(password) + expect(integration).to be_valid + expect(integration.password).to eq(password) end end @@ -432,22 +422,23 @@ RSpec.describe Integrations::Jira do it 'resets password if api url changed' do integration.api_url = 'http://jira_edited.example.com/rest/api/2' - integration.save! + expect(integration).not_to be_valid expect(integration.password).to be_nil end it 'does not reset password if url changed' do integration.url = 'http://jira_edited.example.com' - integration.save! + expect(integration).to be_valid expect(integration.password).to eq(password) end it 'resets password if api url set to empty' do - integration.update!(api_url: '') + integration.api_url = '' - expect(integration.reload.password).to be_nil + expect(integration).not_to be_valid + expect(integration.password).to be_nil end end end @@ -463,8 +454,11 @@ RSpec.describe Integrations::Jira do integration.url = 'http://jira_edited.example.com/rest/api/2' integration.password = 'password' integration.save! - expect(integration.reload.password).to eq('password') - expect(integration.reload.url).to eq('http://jira_edited.example.com/rest/api/2') + + expect(integration.reload).to have_attributes( + url: 'http://jira_edited.example.com/rest/api/2', + password: 'password' + ) end end end @@ -492,7 +486,7 @@ RSpec.describe Integrations::Jira do context 'when data are stored in both properties and separated fields' do let(:properties) { data_params } let(:integration) do - create(:jira_integration, :without_properties_callback, active: false, properties: properties).tap do |integration| + create(:jira_integration, :without_properties_callback, properties: properties).tap do |integration| create(:jira_tracker_data, data_params.merge(integration: integration)) end end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index d425357aef0..0713141ea08 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -76,18 +76,18 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do describe 'Callbacks' do let(:teamcity_integration) { integration } - describe 'before_update :reset_password' do + describe 'before_validation :reset_password' do context 'when a password was previously set' do it 'resets password if url changed' do teamcity_integration.teamcity_url = 'http://gitlab1.com' - teamcity_integration.save! + teamcity_integration.valid? expect(teamcity_integration.password).to be_nil end it 'does not reset password if username changed' do teamcity_integration.username = 'some_name' - teamcity_integration.save! + teamcity_integration.valid? expect(teamcity_integration.password).to eq('password') end @@ -95,7 +95,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do it "does not reset password if new url is set together with password, even if it's the same password" do teamcity_integration.teamcity_url = 'http://gitlab_edited.com' teamcity_integration.password = 'password' - teamcity_integration.save! + teamcity_integration.valid? expect(teamcity_integration.password).to eq('password') expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com') @@ -109,8 +109,10 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do teamcity_integration.password = 'password' teamcity_integration.save! - expect(teamcity_integration.password).to eq('password') - expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com') + expect(teamcity_integration.reload).to have_attributes( + teamcity_url: 'http://gitlab_edited.com', + password: 'password' + ) end end end diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 696b5b48cbf..6aba91d9471 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -88,7 +88,7 @@ RSpec.describe InternalId do context 'when executed outside of transaction' do it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } + allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases expect(InternalId.internal_id_transactions_total).to receive(:increment) .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original @@ -147,7 +147,7 @@ RSpec.describe InternalId do let(:value) { 2 } it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } + allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases expect(InternalId.internal_id_transactions_total).to receive(:increment) .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original @@ -218,7 +218,7 @@ RSpec.describe InternalId do context 'when executed outside of transaction' do it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } + allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases expect(InternalId.internal_id_transactions_total).to receive(:increment) .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 441446bae60..116bda7a18b 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -15,6 +15,7 @@ RSpec.describe Issue do it { is_expected.to belong_to(:iteration) } it { is_expected.to belong_to(:project) } it { is_expected.to have_one(:namespace).through(:project) } + it { is_expected.to belong_to(:work_item_type).class_name('WorkItem::Type') } it { is_expected.to belong_to(:moved_to).class_name('Issue') } it { is_expected.to have_one(:moved_from).class_name('Issue') } it { is_expected.to belong_to(:duplicated_to).class_name('Issue') } @@ -31,6 +32,8 @@ RSpec.describe Issue do it { is_expected.to have_and_belong_to_many(:self_managed_prometheus_alert_events) } it { is_expected.to have_many(:prometheus_alerts) } it { is_expected.to have_many(:issue_email_participants) } + it { is_expected.to have_many(:timelogs).autosave(true) } + it { is_expected.to have_one(:incident_management_issuable_escalation_status) } describe 'versions.most_recent' do it 'returns the most recent version' do @@ -614,33 +617,40 @@ RSpec.describe Issue do let(:subject) { create :issue } end - describe "#to_branch_name" do - let_it_be(:issue) { create(:issue, project: reusable_project, title: 'testing-issue') } + describe '.to_branch_name' do + it 'parameterizes arguments and joins with dashes' do + expect(described_class.to_branch_name(123, 'foo bar', '!@#$%', 'f!o@o#b$a%r^')).to eq('123-foo-bar-f-o-o-b-a-r') + end - it 'starts with the issue iid' do - expect(issue.to_branch_name).to match(/\A#{issue.iid}-[A-Za-z\-]+\z/) + it 'preserves the case in the first argument' do + expect(described_class.to_branch_name('ACME-!@#$-123', 'FoO BaR')).to eq('ACME-123-foo-bar') end - it "contains the issue title if not confidential" do - expect(issue.to_branch_name).to match(/testing-issue\z/) + it 'truncates branch name to at most 100 characters' do + expect(described_class.to_branch_name('a' * 101)).to eq('a' * 100) end - it "does not contain the issue title if confidential" do - issue = create(:issue, project: reusable_project, title: 'testing-issue', confidential: true) - expect(issue.to_branch_name).to match(/confidential-issue\z/) + it 'truncates dangling parts of the branch name' do + branch_name = described_class.to_branch_name( + 999, + 'Lorem ipsum dolor sit amet consectetur adipiscing elit Mauris sit amet ipsum id lacus custom fringilla convallis' + ) + + # 100 characters would've got us "999-lorem...lacus-custom-fri". + expect(branch_name).to eq('999-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-mauris-sit-amet-ipsum-id-lacus-custom') end + end - context 'issue title longer than 100 characters' do - let_it_be(:issue) { create(:issue, project: reusable_project, iid: 999, title: 'Lorem ipsum dolor sit amet consectetur adipiscing elit Mauris sit amet ipsum id lacus custom fringilla convallis') } + describe '#to_branch_name' do + let_it_be(:issue) { create(:issue, project: reusable_project, iid: 123, title: 'Testing Issue') } - it "truncates branch name to at most 100 characters" do - expect(issue.to_branch_name.length).to be <= 100 - end + it 'returns a branch name with the issue title if not confidential' do + expect(issue.to_branch_name).to eq('123-testing-issue') + end - it "truncates dangling parts of the branch name" do - # 100 characters would've got us "999-lorem...lacus-custom-fri". - expect(issue.to_branch_name).to eq("999-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-mauris-sit-amet-ipsum-id-lacus-custom") - end + it 'returns a generic branch name if confidential' do + issue.confidential = true + expect(issue.to_branch_name).to eq('123-confidential-issue') end end @@ -787,17 +797,47 @@ RSpec.describe Issue do end end + shared_examples 'hidden issue readable by user' do + before do + issue.author.ban! + end + + specify do + is_expected.to eq(true) + end + + after do + issue.author.activate! + end + end + + shared_examples 'hidden issue not readable by user' do + before do + issue.author.ban! + end + + specify do + is_expected.to eq(false) + end + + after do + issue.author.activate! + end + end + context 'with an admin user' do let(:user) { build(:admin) } context 'when admin mode is enabled', :enable_admin_mode do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue readable by user' + it_behaves_like 'hidden issue readable by user' end context 'when admin mode is disabled' do it_behaves_like 'issue not readable by user' it_behaves_like 'confidential issue not readable by user' + it_behaves_like 'hidden issue not readable by user' end end @@ -808,6 +848,7 @@ RSpec.describe Issue do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue readable by user' + it_behaves_like 'hidden issue not readable by user' end context 'with a reporter user' do @@ -817,6 +858,7 @@ RSpec.describe Issue do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue readable by user' + it_behaves_like 'hidden issue not readable by user' end context 'with a guest user' do @@ -826,6 +868,7 @@ RSpec.describe Issue do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue not readable by user' + it_behaves_like 'hidden issue not readable by user' context 'when user is an assignee' do before do @@ -834,6 +877,7 @@ RSpec.describe Issue do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue readable by user' + it_behaves_like 'hidden issue not readable by user' end context 'when user is the author' do @@ -843,6 +887,7 @@ RSpec.describe Issue do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue readable by user' + it_behaves_like 'hidden issue not readable by user' end end @@ -852,6 +897,7 @@ RSpec.describe Issue do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue not readable by user' + it_behaves_like 'hidden issue not readable by user' end context 'using an internal project' do @@ -864,6 +910,7 @@ RSpec.describe Issue do it_behaves_like 'issue readable by user' it_behaves_like 'confidential issue not readable by user' + it_behaves_like 'hidden issue not readable by user' end context 'using an external user' do @@ -873,6 +920,7 @@ RSpec.describe Issue do it_behaves_like 'issue not readable by user' it_behaves_like 'confidential issue not readable by user' + it_behaves_like 'hidden issue not readable by user' end end @@ -883,6 +931,7 @@ RSpec.describe Issue do it_behaves_like 'issue not readable by user' it_behaves_like 'confidential issue not readable by user' + it_behaves_like 'hidden issue not readable by user' end end @@ -1112,14 +1161,14 @@ RSpec.describe Issue do with_them do it 'checks for spam when necessary' do - author = support_bot? ? support_bot : user + active_user = support_bot? ? support_bot : user project = reusable_project project.update!(visibility_level: visibility_level) - issue = create(:issue, project: project, confidential: confidential, description: 'original description', author: author) + issue = create(:issue, project: project, confidential: confidential, description: 'original description', author: support_bot) issue.assign_attributes(new_attributes) - expect(issue.check_for_spam?).to eq(check_for_spam?) + expect(issue.check_for_spam?(user: active_user)).to eq(check_for_spam?) end end end @@ -1151,6 +1200,26 @@ RSpec.describe Issue do end end + describe '.without_hidden' do + let_it_be(:banned_user) { create(:user, :banned) } + let_it_be(:public_issue) { create(:issue, project: reusable_project) } + let_it_be(:hidden_issue) { create(:issue, project: reusable_project, author: banned_user) } + + it 'only returns without_hidden issues' do + expect(described_class.without_hidden).to eq([public_issue]) + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(ban_user_feature_flag: false) + end + + it 'returns public and hidden issues' do + expect(described_class.without_hidden).to eq([public_issue, hidden_issue]) + end + end + end + describe '.by_project_id_and_iid' do let_it_be(:issue_a) { create(:issue, project: reusable_project) } let_it_be(:issue_b) { create(:issue, iid: issue_a.iid) } diff --git a/spec/models/jira_connect_installation_spec.rb b/spec/models/jira_connect_installation_spec.rb index 8ef96114c45..3d1095845aa 100644 --- a/spec/models/jira_connect_installation_spec.rb +++ b/spec/models/jira_connect_installation_spec.rb @@ -15,6 +15,9 @@ RSpec.describe JiraConnectInstallation do it { is_expected.to allow_value('https://test.atlassian.net').for(:base_url) } it { is_expected.not_to allow_value('not/a/url').for(:base_url) } + + it { is_expected.to allow_value('https://test.atlassian.net').for(:instance_url) } + it { is_expected.not_to allow_value('not/a/url').for(:instance_url) } end describe '.for_project' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 5824c2085ce..067b3c25645 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -64,6 +64,49 @@ RSpec.describe Member do end end + context 'with admin signup restrictions' do + context 'when allowed domains for signup is enabled' do + before do + stub_application_setting(domain_allowlist: ['example.com']) + end + + it 'adds an error message when email is not accepted' do + member = build(:group_member, :invited, invite_email: 'info@gitlab.com') + + expect(member).not_to be_valid + expect(member.errors.messages[:user].first).to eq(_('domain is not authorized for sign-up.')) + end + end + + context 'when denylist is enabled' do + before do + stub_application_setting(domain_denylist_enabled: true) + stub_application_setting(domain_denylist: ['example.org']) + end + + it 'adds an error message when email is denied' do + member = build(:group_member, :invited, invite_email: 'denylist@example.org') + + expect(member).not_to be_valid + expect(member.errors.messages[:user].first).to eq(_('is not from an allowed domain.')) + end + end + + context 'when email restrictions is enabled' do + before do + stub_application_setting(email_restrictions_enabled: true) + stub_application_setting(email_restrictions: '([\+]|\b(\w*gitlab.com\w*)\b)') + end + + it 'adds an error message when email is not accepted' do + member = build(:group_member, :invited, invite_email: 'info@gitlab.com') + + expect(member).not_to be_valid + expect(member.errors.messages[:user].first).to eq(_('is not allowed. Try again with a different email address, or contact your GitLab admin.')) + end + end + end + context "when a child member inherits its access level" do let(:user) { create(:user) } let(:member) { create(:group_member, :developer, user: user) } @@ -624,7 +667,23 @@ RSpec.describe Member do let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } it "sets the invite token" do - expect { member.generate_invite_token }.to change { member.invite_token} + expect { member.generate_invite_token }.to change { member.invite_token } + end + end + + describe 'generate invite token on create' do + let!(:member) { build(:project_member, invite_email: "user@example.com") } + + it "sets the invite token" do + expect { member.save! }.to change { member.invite_token }.to(kind_of(String)) + end + + context 'when invite was already accepted' do + it "does not set invite token" do + member.invite_accepted_at = 1.day.ago + + expect { member.save! }.not_to change { member.invite_token }.from(nil) + end end end @@ -749,4 +808,44 @@ RSpec.describe Member do end end end + + describe 'log_invitation_token_cleanup' do + let_it_be(:project) { create :project } + + context 'when on gitlab.com' do + before do + allow(Gitlab).to receive(:com?).and_return true + end + + it "doesn't log info for members without invitation or accepted invitation" do + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + + create :project_member + create :project_member, :invited, invite_accepted_at: nil + create :project_member, invite_token: nil, invite_accepted_at: Time.zone.now + end + + it 'logs error for accepted members with token and creates membership' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(kind_of(StandardError), kind_of(Hash)) + + expect do + create :project_member, :invited, source: project, invite_accepted_at: Time.zone.now + end.to change { Member.count }.by(1) + end + end + + context 'when not on gitlab.com' do + before do + allow(Gitlab).to receive(:com?).and_return false + end + + it 'does not log error for accepted members with token and creates membership' do + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + + expect do + create :project_member, :invited, source: project, invite_accepted_at: Time.zone.now + end.to change { Member.count }.by(1) + end + end + end end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 472f4280d26..92f9099d04d 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -136,4 +136,16 @@ RSpec.describe GroupMember do group_member.update!(expires_at: 5.days.from_now) end end + + describe 'refresh_member_authorized_projects' do + context 'when importing' do + it 'does not refresh' do + expect(UserProjectAccessChangedService).not_to receive(:new) + + member = build(:group_member) + member.importing = true + member.save! + end + end + end end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 4c59bda856f..1704d5adb96 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -139,4 +139,171 @@ RSpec.describe ProjectMember do end end end + + context 'refreshing project_authorizations' do + let_it_be_with_refind(:project) { create(:project) } + let_it_be_with_refind(:user) { create(:user) } + let_it_be(:project_member) { create(:project_member, :guest, project: project, user: user) } + + context 'when the source project of the project member is destroyed' do + it 'refreshes the authorization of user to the project in the group' do + expect { project.destroy! }.to change { user.can?(:guest_access, project) }.from(true).to(false) + end + + it 'refreshes the authorization without calling AuthorizedProjectUpdate::ProjectRecalculatePerUserService' do + expect(AuthorizedProjectUpdate::ProjectRecalculatePerUserService).not_to receive(:new) + + project.destroy! + end + end + + context 'when the user of the project member is destroyed' do + it 'refreshes the authorization of user to the project in the group' do + expect(project.authorized_users).to include(user) + + user.destroy! + + expect(project.authorized_users).not_to include(user) + end + + it 'refreshes the authorization without calling UserProjectAccessChangedService' do + expect(UserProjectAccessChangedService).not_to receive(:new) + + user.destroy! + end + end + + context 'when importing' do + it 'does not refresh' do + expect(AuthorizedProjectUpdate::ProjectRecalculatePerUserService).not_to receive(:new) + + member = build(:project_member) + member.importing = true + member.save! + end + end + end + + context 'authorization refresh on addition/updation/deletion' do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + shared_examples_for 'calls AuthorizedProjectUpdate::ProjectRecalculatePerUserService to recalculate authorizations' do + it 'calls AuthorizedProjectUpdate::ProjectRecalculatePerUserService' do + expect_next_instance_of(AuthorizedProjectUpdate::ProjectRecalculatePerUserService, project, user) do |service| + expect(service).to receive(:execute) + end + + action + end + end + + shared_examples_for 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' do + it 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker' do + expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( + receive(:bulk_perform_in) + .with(1.hour, + [[user.id]], + batch_delay: 30.seconds, batch_size: 100) + ) + + action + end + end + + context 'on create' do + let(:action) { project.add_user(user, Gitlab::Access::GUEST) } + + it 'changes access level' do + expect { action }.to change { user.can?(:guest_access, project) }.from(false).to(true) + end + + it_behaves_like 'calls AuthorizedProjectUpdate::ProjectRecalculatePerUserService to recalculate authorizations' + it_behaves_like 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' + end + + context 'on update' do + let(:action) { project.members.find_by(user: user).update!(access_level: Gitlab::Access::DEVELOPER) } + + before do + project.add_user(user, Gitlab::Access::GUEST) + end + + it 'changes access level' do + expect { action }.to change { user.can?(:developer_access, project) }.from(false).to(true) + end + + it_behaves_like 'calls AuthorizedProjectUpdate::ProjectRecalculatePerUserService to recalculate authorizations' + it_behaves_like 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' + end + + context 'on destroy' do + let(:action) { project.members.find_by(user: user).destroy! } + + before do + project.add_user(user, Gitlab::Access::GUEST) + end + + it 'changes access level' do + expect { action }.to change { user.can?(:guest_access, project) }.from(true).to(false) + end + + it_behaves_like 'calls AuthorizedProjectUpdate::ProjectRecalculatePerUserService to recalculate authorizations' + it_behaves_like 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' + end + + context 'when the feature flag `specialized_service_for_project_member_auth_refresh` is disabled' do + before do + stub_feature_flags(specialized_service_for_project_member_auth_refresh: false) + end + + shared_examples_for 'calls UserProjectAccessChangedService to recalculate authorizations' do + it 'calls UserProjectAccessChangedService' do + expect_next_instance_of(UserProjectAccessChangedService, user.id) do |service| + expect(service).to receive(:execute) + end + + action + end + end + + context 'on create' do + let(:action) { project.add_user(user, Gitlab::Access::GUEST) } + + it 'changes access level' do + expect { action }.to change { user.can?(:guest_access, project) }.from(false).to(true) + end + + it_behaves_like 'calls UserProjectAccessChangedService to recalculate authorizations' + end + + context 'on update' do + let(:action) { project.members.find_by(user: user).update!(access_level: Gitlab::Access::DEVELOPER) } + + before do + project.add_user(user, Gitlab::Access::GUEST) + end + + it 'changes access level' do + expect { action }.to change { user.can?(:developer_access, project) }.from(false).to(true) + end + + it_behaves_like 'calls UserProjectAccessChangedService to recalculate authorizations' + end + + context 'on destroy' do + let(:action) { project.members.find_by(user: user).destroy! } + + before do + project.add_user(user, Gitlab::Access::GUEST) + end + + it 'changes access level' do + expect { action }.to change { user.can?(:guest_access, project) }.from(true).to(false) + end + + it_behaves_like 'calls UserProjectAccessChangedService to recalculate authorizations' + end + end + end end diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb index 6290468d4a7..adddec7ced8 100644 --- a/spec/models/merge_request_diff_commit_spec.rb +++ b/spec/models/merge_request_diff_commit_spec.rb @@ -79,7 +79,7 @@ RSpec.describe MergeRequestDiffCommit do subject { described_class.create_bulk(merge_request_diff_id, commits) } it 'inserts the commits into the database en masse' do - expect(Gitlab::Database).to receive(:bulk_insert) + expect(Gitlab::Database.main).to receive(:bulk_insert) .with(described_class.table_name, rows) subject @@ -126,7 +126,7 @@ RSpec.describe MergeRequestDiffCommit do end it 'uses a sanitized date' do - expect(Gitlab::Database).to receive(:bulk_insert) + expect(Gitlab::Database.main).to receive(:bulk_insert) .with(described_class.table_name, rows) subject diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e0e25031589..5fff880c44e 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -240,7 +240,7 @@ RSpec.describe MergeRequestDiff do stub_external_diffs_setting(enabled: true) expect(diff).not_to receive(:save!) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with('merge_request_diff_files', anything) .and_raise(ActiveRecord::Rollback) @@ -465,11 +465,13 @@ RSpec.describe MergeRequestDiff do it 'sorts diff files directory first' do diff_with_commits.update!(sorted: false) # Mark as unsorted so it'll re-order - expect(diff_with_commits.diffs_in_batch(0, 10, diff_options: diff_options).diff_file_paths).to eq([ + # There will be 11 returned, as we have to take into account for new and old paths + expect(diff_with_commits.diffs_in_batch(0, 10, diff_options: diff_options).diff_paths).to eq([ 'bar/branch-test.txt', 'custom-highlighting/test.gitlab-custom', 'encoding/iso8859.txt', 'files/images/wm.svg', + 'files/js/commit.js.coffee', 'files/js/commit.coffee', 'files/lfs/lfs_object.iso', 'files/ruby/popen.rb', @@ -553,11 +555,12 @@ RSpec.describe MergeRequestDiff do it 'sorts diff files directory first' do diff_with_commits.update!(sorted: false) # Mark as unsorted so it'll re-order - expect(diff_with_commits.diffs(diff_options).diff_file_paths).to eq([ + expect(diff_with_commits.diffs(diff_options).diff_paths).to eq([ 'bar/branch-test.txt', 'custom-highlighting/test.gitlab-custom', 'encoding/iso8859.txt', 'files/images/wm.svg', + 'files/js/commit.js.coffee', 'files/js/commit.coffee', 'files/lfs/lfs_object.iso', 'files/ruby/popen.rb', diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index edd543854cb..4a8a2909891 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -80,6 +80,24 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '.order_closed_at_asc' do + let_it_be(:older_mr) { create(:merge_request, :closed_last_month) } + let_it_be(:newer_mr) { create(:merge_request, :closed_last_month) } + + it 'returns MRs ordered by closed_at ascending' do + expect(described_class.order_closed_at_asc).to eq([older_mr, newer_mr]) + end + end + + describe '.order_closed_at_desc' do + let_it_be(:older_mr) { create(:merge_request, :closed_last_month) } + let_it_be(:newer_mr) { create(:merge_request, :closed_last_month) } + + it 'returns MRs ordered by closed_at descending' do + expect(described_class.order_closed_at_desc).to eq([newer_mr, older_mr]) + end + end + describe '.with_jira_issue_keys' do let_it_be(:mr_with_jira_title) { create(:merge_request, :unique_branches, title: 'Fix TEST-123') } let_it_be(:mr_with_jira_description) { create(:merge_request, :unique_branches, description: 'this closes TEST-321') } @@ -577,6 +595,26 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(merge_requests).to eq([newer_mr, older_mr]) end end + + context 'closed_at' do + let_it_be(:older_mr) { create(:merge_request, :closed_last_month) } + let_it_be(:newer_mr) { create(:merge_request, :closed_last_month) } + + it 'sorts asc' do + merge_requests = described_class.sort_by_attribute(:closed_at_asc) + expect(merge_requests).to eq([older_mr, newer_mr]) + end + + it 'sorts desc' do + merge_requests = described_class.sort_by_attribute(:closed_at_desc) + expect(merge_requests).to eq([newer_mr, older_mr]) + end + + it 'sorts asc when its closed_at' do + merge_requests = described_class.sort_by_attribute(:closed_at) + expect(merge_requests).to eq([older_mr, newer_mr]) + end + end end describe 'time to merge calculations' do diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index bc592acc80f..f14b9c57eb1 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -538,6 +538,15 @@ RSpec.describe Milestone do it { is_expected.to match('gitlab-org/gitlab-ce%123') } it { is_expected.to match('gitlab-org/gitlab-ce%"my-milestone"') } + + context 'when milestone_reference_pattern feature flag is false' do + before do + stub_feature_flags(milestone_reference_pattern: false) + end + + it { is_expected.to match('gitlab-org/gitlab-ce%123') } + it { is_expected.to match('gitlab-org/gitlab-ce%"my-milestone"') } + end end describe '.link_reference_pattern' do diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 14d28be8d43..e8ed6f1a460 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -41,6 +41,14 @@ RSpec.describe NamespaceSetting, type: :model do it_behaves_like "doesn't return an error" end + + context "when it contains javascript tags" do + it "gets sanitized properly" do + namespace_settings.update!(default_branch_name: "hello<script>alert(1)</script>") + + expect(namespace_settings.default_branch_name).to eq('hello') + end + end end describe '#allow_mfa_for_group' do @@ -98,4 +106,81 @@ RSpec.describe NamespaceSetting, type: :model do end end end + + describe '#prevent_sharing_groups_outside_hierarchy' do + let(:settings) { create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true) } + let!(:group) { create(:group, parent: parent, namespace_settings: settings ) } + + subject(:group_sharing_setting) { settings.prevent_sharing_groups_outside_hierarchy } + + context 'when this namespace is a root ancestor' do + let(:parent) { nil } + + it 'returns the actual stored value' do + expect(group_sharing_setting).to be_truthy + end + end + + context 'when this namespace is a descendant' do + let(:parent) { create(:group) } + + it 'returns the value stored for the parent settings' do + expect(group_sharing_setting).to eq(parent.namespace_settings.prevent_sharing_groups_outside_hierarchy) + expect(group_sharing_setting).to be_falsey + end + end + end + + describe 'hooks related to group user cap update' do + let(:settings) { create(:namespace_settings, new_user_signups_cap: user_cap) } + let(:group) { create(:group, namespace_settings: settings) } + + before do + allow(group).to receive(:root?).and_return(true) + end + + context 'when updating a group with a user cap' do + let(:user_cap) { nil } + + it 'also sets share_with_group_lock and prevent_sharing_groups_outside_hierarchy to true' do + expect(group.new_user_signups_cap).to be_nil + expect(group.share_with_group_lock).to be_falsey + expect(settings.prevent_sharing_groups_outside_hierarchy).to be_falsey + + settings.update!(new_user_signups_cap: 10) + group.reload + + expect(group.new_user_signups_cap).to eq(10) + expect(group.share_with_group_lock).to be_truthy + expect(settings.reload.prevent_sharing_groups_outside_hierarchy).to be_truthy + end + + it 'has share_with_group_lock and prevent_sharing_groups_outside_hierarchy returning true for descendent groups' do + descendent = create(:group, parent: group) + desc_settings = descendent.namespace_settings + + expect(descendent.share_with_group_lock).to be_falsey + expect(desc_settings.prevent_sharing_groups_outside_hierarchy).to be_falsey + + settings.update!(new_user_signups_cap: 10) + + expect(descendent.reload.share_with_group_lock).to be_truthy + expect(desc_settings.reload.prevent_sharing_groups_outside_hierarchy).to be_truthy + end + end + + context 'when removing a user cap from namespace settings' do + let(:user_cap) { 10 } + + it 'leaves share_with_group_lock and prevent_sharing_groups_outside_hierarchy set to true to the related group' do + expect(group.share_with_group_lock).to be_truthy + expect(settings.prevent_sharing_groups_outside_hierarchy).to be_truthy + + settings.update!(new_user_signups_cap: nil) + + expect(group.reload.share_with_group_lock).to be_truthy + expect(settings.reload.prevent_sharing_groups_outside_hierarchy).to be_truthy + end + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index ea1ce067e4d..e2700378f5f 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -23,6 +23,7 @@ RSpec.describe Namespace do it { is_expected.to have_one :package_setting_relation } it { is_expected.to have_one :onboarding_progress } it { is_expected.to have_one :admin_note } + it { is_expected.to have_many :pending_builds } end describe 'validations' do @@ -207,9 +208,23 @@ RSpec.describe Namespace do it { is_expected.to include_module(Gitlab::VisibilityLevel) } it { is_expected.to include_module(Namespaces::Traversal::Recursive) } it { is_expected.to include_module(Namespaces::Traversal::Linear) } + it { is_expected.to include_module(Namespaces::Traversal::RecursiveScopes) } + it { is_expected.to include_module(Namespaces::Traversal::LinearScopes) } end - it_behaves_like 'linear namespace traversal' + context 'traversal scopes' do + context 'recursive' do + before do + stub_feature_flags(use_traversal_ids: false) + end + + it_behaves_like 'namespace traversal scopes' + end + + context 'linear' do + it_behaves_like 'namespace traversal scopes' + end + end context 'traversal_ids on create' do context 'default traversal_ids' do @@ -1152,6 +1167,68 @@ RSpec.describe Namespace do end end + context 'refreshing project access on updating share_with_group_lock' do + let(:group) { create(:group, share_with_group_lock: false) } + let(:project) { create(:project, :private, group: group) } + + let_it_be(:shared_with_group_one) { create(:group) } + let_it_be(:shared_with_group_two) { create(:group) } + let_it_be(:group_one_user) { create(:user) } + let_it_be(:group_two_user) { create(:user) } + + subject(:execute_update) { group.update!(share_with_group_lock: true) } + + before do + shared_with_group_one.add_developer(group_one_user) + shared_with_group_two.add_developer(group_two_user) + create(:project_group_link, group: shared_with_group_one, project: project) + create(:project_group_link, group: shared_with_group_two, project: project) + end + + it 'calls AuthorizedProjectUpdate::ProjectRecalculateWorker to update project authorizations' do + expect(AuthorizedProjectUpdate::ProjectRecalculateWorker) + .to receive(:perform_async).with(project.id).once + + execute_update + end + + it 'updates authorizations leading to users from shared groups losing access', :sidekiq_inline do + expect { execute_update } + .to change { group_one_user.authorized_projects.include?(project) }.from(true).to(false) + .and change { group_two_user.authorized_projects.include?(project) }.from(true).to(false) + end + + it 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' do + expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( + receive(:bulk_perform_in) + .with(1.hour, + [[group_one_user.id]], + batch_delay: 30.seconds, batch_size: 100) + ) + + expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( + receive(:bulk_perform_in) + .with(1.hour, + [[group_two_user.id]], + batch_delay: 30.seconds, batch_size: 100) + ) + + execute_update + end + + context 'when the feature flag `specialized_worker_for_group_lock_update_auth_recalculation` is disabled' do + before do + stub_feature_flags(specialized_worker_for_group_lock_update_auth_recalculation: false) + end + + it 'refreshes the permissions of the members of the old and new namespace' do + expect { execute_update } + .to change { group_one_user.authorized_projects.include?(project) }.from(true).to(false) + .and change { group_two_user.authorized_projects.include?(project) }.from(true).to(false) + end + end + end + describe '#share_with_group_lock with subgroups' do context 'when creating a subgroup' do let(:subgroup) { create(:group, parent: root_group )} diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 2afe9a0f29b..0afdae2fc93 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1538,4 +1538,24 @@ RSpec.describe Note do expect(attachment).not_to be_exist end end + + describe '#post_processed_cache_key' do + let(:note) { build(:note) } + + it 'returns cache key and author cache key by default' do + expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}") + end + + context 'when note has redacted_note_html' do + let(:redacted_note_html) { 'redacted note html' } + + before do + note.redacted_note_html = redacted_note_html + end + + it 'returns cache key with redacted_note_html sha' do + expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}:#{Digest::SHA1.hexdigest(redacted_note_html)}") + end + end + end end diff --git a/spec/models/operations/feature_flags/strategy_spec.rb b/spec/models/operations/feature_flags/strategy_spec.rb index 0ecb49e75f3..9289e3beab5 100644 --- a/spec/models/operations/feature_flags/strategy_spec.rb +++ b/spec/models/operations/feature_flags/strategy_spec.rb @@ -112,7 +112,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do end context 'when the strategy name is flexibleRollout' do - valid_parameters = { rollout: '40', groupId: 'mygroup', stickiness: 'DEFAULT' } + valid_parameters = { rollout: '40', groupId: 'mygroup', stickiness: 'default' } where(invalid_parameters: [ nil, {}, @@ -133,7 +133,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do [ [:rollout, '10'], - [:stickiness, 'DEFAULT'], + [:stickiness, 'default'], [:groupId, 'mygroup'] ].permutation(3).each do |parameters| it "allows the parameters in the order #{parameters.map { |p| p.first }.join(', ')}" do @@ -151,7 +151,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do "\n", "\t", "\n10", "20\n", "\n100", "100\n", "\n ", nil]) with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: invalid_value } + parameters = { stickiness: 'default', groupId: 'mygroup', rollout: invalid_value } strategy = described_class.create(feature_flag: feature_flag, name: 'flexibleRollout', parameters: parameters) @@ -165,7 +165,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do where(valid_value: %w[0 1 10 38 100 93]) with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: valid_value } + parameters = { stickiness: 'default', groupId: 'mygroup', rollout: valid_value } strategy = described_class.create(feature_flag: feature_flag, name: 'flexibleRollout', parameters: parameters) @@ -180,7 +180,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do '!bad', '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"]) with_them do it 'must be a string value of up to 32 lowercase characters' do - parameters = { stickiness: 'DEFAULT', groupId: invalid_value, rollout: '40' } + parameters = { stickiness: 'default', groupId: invalid_value, rollout: '40' } strategy = described_class.create(feature_flag: feature_flag, name: 'flexibleRollout', parameters: parameters) @@ -192,7 +192,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do where(valid_value: ["somegroup", "anothergroup", "okay", "g", "a" * 32]) with_them do it 'must be a string value of up to 32 lowercase characters' do - parameters = { stickiness: 'DEFAULT', groupId: valid_value, rollout: '40' } + parameters = { stickiness: 'default', groupId: valid_value, rollout: '40' } strategy = described_class.create(feature_flag: feature_flag, name: 'flexibleRollout', parameters: parameters) @@ -203,7 +203,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do end describe 'stickiness' do - where(invalid_value: [nil, " ", "default", "DEFAULT\n", "UserId", "USER", "USERID "]) + where(invalid_value: [nil, " ", "DEFAULT", "DEFAULT\n", "UserId", "USER", "USERID "]) with_them do it 'must be a string representing a supported stickiness setting' do parameters = { stickiness: invalid_value, groupId: 'mygroup', rollout: '40' } @@ -212,12 +212,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do parameters: parameters) expect(strategy.errors[:parameters]).to eq([ - 'stickiness parameter must be DEFAULT, USERID, SESSIONID, or RANDOM' + 'stickiness parameter must be default, userId, sessionId, or random' ]) end end - where(valid_value: %w[DEFAULT USERID SESSIONID RANDOM]) + where(valid_value: %w[default userId sessionId random]) with_them do it 'must be a string representing a supported stickiness setting' do parameters = { stickiness: valid_value, groupId: 'mygroup', rollout: '40' } @@ -425,7 +425,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do user_list: user_list, parameters: { groupId: 'default', rollout: '10', - stickiness: 'DEFAULT' }) + stickiness: 'default' }) expect(strategy.errors[:user_list]).to eq(['must be blank']) end @@ -435,7 +435,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do name: 'flexibleRollout', parameters: { groupId: 'default', rollout: '10', - stickiness: 'DEFAULT' }) + stickiness: 'default' }) expect(strategy.errors[:user_list]).to be_empty end diff --git a/spec/models/packages/npm_spec.rb b/spec/models/packages/npm_spec.rb new file mode 100644 index 00000000000..fa4adadfe06 --- /dev/null +++ b/spec/models/packages/npm_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Packages::Npm do + using RSpec::Parameterized::TableSyntax + + describe '.scope_of' do + subject { described_class.scope_of(package_name) } + + where(:package_name, :expected_result) do + nil | nil + 'test' | nil + '@test' | nil + 'test/package' | nil + '@/package' | nil + '@test/package' | 'test' + '@test/' | nil + end + + with_them do + it { is_expected.to eq(expected_result) } + end + end +end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index ee0aeb26d50..90910fcb7ce 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -159,4 +159,71 @@ RSpec.describe Packages::PackageFile, type: :model do expect { subject }.to change { package_file.size }.from(nil).to(3513) end end + + context 'update callbacks' do + subject { package_file.save! } + + shared_examples 'executing the default callback' do + it 'executes the default callback' do + expect(package_file).to receive(:remove_previously_stored_file) + expect(package_file).not_to receive(:move_in_object_storage) + + subject + end + end + + context 'with object storage disabled' do + let(:package_file) { create(:package_file, file_name: 'file_name.txt') } + + before do + stub_package_file_object_storage(enabled: false) + end + + it_behaves_like 'executing the default callback' + + context 'with new_file_path set' do + before do + package_file.new_file_path = 'test' + end + + it_behaves_like 'executing the default callback' + end + end + + context 'with object storage enabled' do + let(:package_file) do + create( + :package_file, + file_name: 'file_name.txt', + file: CarrierWaveStringFile.new_file( + file_content: 'content', + filename: 'file_name.txt', + content_type: 'text/plain' + ), + file_store: ::Packages::PackageFileUploader::Store::REMOTE + ) + end + + before do + stub_package_file_object_storage(enabled: true) + end + + it_behaves_like 'executing the default callback' + + context 'with new_file_path set' do + before do + package_file.new_file_path = 'test' + end + + it 'executes the move_in_object_storage callback' do + expect(package_file).not_to receive(:remove_previously_stored_file) + expect(package_file).to receive(:move_in_object_storage).and_call_original + expect(package_file.file.file).to receive(:copy_to).and_call_original + expect(package_file.file.file).to receive(:delete).and_call_original + + subject + end + end + end + end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 449e30f9fb7..4d4d4ad4fa9 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe Packages::Package, type: :model do include SortingHelper + using RSpec::Parameterized::TableSyntax it_behaves_like 'having unique enum values' @@ -418,7 +419,7 @@ RSpec.describe Packages::Package, type: :model do end end - describe '#package_already_taken' do + describe '#npm_package_already_taken' do context 'maven package' do let!(:package) { create(:maven_package) } @@ -428,6 +429,164 @@ RSpec.describe Packages::Package, type: :model do expect(new_package).to be_valid end end + + context 'npm package' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } + let_it_be(:second_project) { create(:project, namespace: group)} + + let(:package) { build(:npm_package, project: project, name: name) } + + shared_examples 'validating the first package' do + it 'validates the first package' do + expect(package).to be_valid + end + end + + shared_examples 'validating the second package' do + it 'validates the second package' do + package.save! + + expect(second_package).to be_valid + end + end + + shared_examples 'not validating the second package' do |field_with_error:| + it 'does not validate the second package' do + package.save! + + expect(second_package).not_to be_valid + case field_with_error + when :base + expect(second_package.errors.messages[:base]).to eq ['Package already exists'] + when :name + expect(second_package.errors.messages[:name]).to eq ['has already been taken'] + else + raise ArgumentError, "field #{field_with_error} not expected" + end + end + end + + context 'following the naming convention' do + let(:name) { "@#{group.path}/test" } + + context 'with the second package in the project of the first package' do + let(:second_package) { build(:npm_package, project: project, name: second_package_name, version: second_package_version) } + + context 'with no duplicated name' do + let(:second_package_name) { "@#{group.path}/test2" } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicated name' do + let(:second_package_name) { package.name } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicate name and duplicated version' do + let(:second_package_name) { package.name } + let(:second_package_version) { package.version } + + it_behaves_like 'validating the first package' + it_behaves_like 'not validating the second package', field_with_error: :name + end + end + + context 'with the second package in a different project than the first package' do + let(:second_package) { build(:npm_package, project: second_project, name: second_package_name, version: second_package_version) } + + context 'with no duplicated name' do + let(:second_package_name) { "@#{group.path}/test2" } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicated name' do + let(:second_package_name) { package.name } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicate name and duplicated version' do + let(:second_package_name) { package.name } + let(:second_package_version) { package.version } + + it_behaves_like 'validating the first package' + it_behaves_like 'not validating the second package', field_with_error: :base + end + end + end + + context 'not following the naming convention' do + let(:name) { '@foobar/test' } + + context 'with the second package in the project of the first package' do + let(:second_package) { build(:npm_package, project: project, name: second_package_name, version: second_package_version) } + + context 'with no duplicated name' do + let(:second_package_name) { "@foobar/test2" } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicated name' do + let(:second_package_name) { package.name } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicate name and duplicated version' do + let(:second_package_name) { package.name } + let(:second_package_version) { package.version } + + it_behaves_like 'validating the first package' + it_behaves_like 'not validating the second package', field_with_error: :name + end + end + + context 'with the second package in a different project than the first package' do + let(:second_package) { build(:npm_package, project: second_project, name: second_package_name, version: second_package_version) } + + context 'with no duplicated name' do + let(:second_package_name) { "@foobar/test2" } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicated name' do + let(:second_package_name) { package.name } + let(:second_package_version) { '5.0.0' } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + + context 'with duplicate name and duplicated version' do + let(:second_package_name) { package.name } + let(:second_package_version) { package.version } + + it_behaves_like 'validating the first package' + it_behaves_like 'validating the second package' + end + end + end + end end context "recipe uniqueness for conan packages" do diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 67ecbe13c1a..8cd831d2f85 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -73,6 +73,14 @@ RSpec.describe PersonalAccessToken do end end + describe '#expired_but_not_enforced?' do + let(:token) { build(:personal_access_token) } + + it 'returns false', :aggregate_failures do + expect(token).not_to be_expired_but_not_enforced + end + end + describe 'Redis storage' do let(:user_id) { 123 } let(:token) { 'KS3wegQYXBLYhQsciwsj' } diff --git a/spec/models/postgresql/detached_partition_spec.rb b/spec/models/postgresql/detached_partition_spec.rb new file mode 100644 index 00000000000..aaa99e842b4 --- /dev/null +++ b/spec/models/postgresql/detached_partition_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Postgresql::DetachedPartition do + describe '#ready_to_drop' do + let_it_be(:drop_before) { Postgresql::DetachedPartition.create!(drop_after: 1.day.ago, table_name: 'old_table') } + let_it_be(:drop_after) { Postgresql::DetachedPartition.create!(drop_after: 1.day.from_now, table_name: 'new_table') } + + it 'includes partitions that should be dropped before now' do + expect(Postgresql::DetachedPartition.ready_to_drop.to_a).to include(drop_before) + end + + it 'does not include partitions that should be dropped after now' do + expect(Postgresql::DetachedPartition.ready_to_drop.to_a).not_to include(drop_after) + end + end +end diff --git a/spec/models/postgresql/replication_slot_spec.rb b/spec/models/postgresql/replication_slot_spec.rb index 4bad8a3f0c0..c3b67a2e7b8 100644 --- a/spec/models/postgresql/replication_slot_spec.rb +++ b/spec/models/postgresql/replication_slot_spec.rb @@ -60,4 +60,71 @@ RSpec.describe Postgresql::ReplicationSlot do expect(described_class.lag_too_great?).to eq(false) end end + + describe '#max_replication_slots' do + it 'returns the maximum number of replication slots' do + expect(described_class.max_replication_slots).to be >= 0 + end + end + + context 'with enough slots available' do + skip_examples = described_class.max_replication_slots <= described_class.count + + before(:all) do + skip('max_replication_slots too small') if skip_examples + + @current_slot_count = ApplicationRecord + .connection + .execute("SELECT COUNT(*) FROM pg_replication_slots;") + .first + .fetch('count') + .to_i + + @current_unused_count = ApplicationRecord + .connection + .execute("SELECT COUNT(*) FROM pg_replication_slots WHERE active = 'f';") + .first + .fetch('count') + .to_i + + ApplicationRecord + .connection + .execute("SELECT * FROM pg_create_physical_replication_slot('test_slot');") + end + + after(:all) do + unless skip_examples + ApplicationRecord + .connection + .execute("SELECT pg_drop_replication_slot('test_slot');") + end + end + + describe '#slots_count' do + it 'returns the number of replication slots' do + expect(described_class.count).to eq(@current_slot_count + 1) + end + end + + describe '#unused_slots_count' do + it 'returns the number of unused replication slots' do + expect(described_class.unused_slots_count).to eq(@current_unused_count + 1) + end + end + + describe '#max_retained_wal' do + it 'returns the retained WAL size' do + expect(described_class.max_retained_wal).not_to be_nil + end + end + + describe '#slots_retained_bytes' do + it 'returns the number of retained bytes' do + slot = described_class.slots_retained_bytes.find {|x| x['slot_name'] == 'test_slot' } + + expect(slot).not_to be_nil + expect(slot['retained_bytes']).to be_nil + end + end + end end diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 3fd7e57a5db..5f720f8c4f8 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -8,6 +8,8 @@ RSpec.describe ProjectFeature do let(:project) { create(:project) } let(:user) { create(:user) } + it { is_expected.to belong_to(:project) } + describe 'PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT' do it 'has higher level than that of PRIVATE_FEATURES_MIN_ACCESS_LEVEL' do described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.each do |feature, level| @@ -189,27 +191,57 @@ RSpec.describe ProjectFeature do end describe 'container_registry_access_level' do - context 'when the project is created with container_registry_enabled false' do - it 'creates project with DISABLED container_registry_access_level' do - project = create(:project, container_registry_enabled: false) + context 'with default value' do + let(:project) { Project.new } + + context 'when the default is false' do + it 'creates project_feature with `disabled` container_registry_access_level' do + stub_config_setting(default_projects_features: { container_registry: false }) - expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED) + expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED) + end end - end - context 'when the project is created with container_registry_enabled true' do - it 'creates project with ENABLED container_registry_access_level' do - project = create(:project, container_registry_enabled: true) + context 'when the default is true' do + before do + stub_config_setting(default_projects_features: { container_registry: true }) + end - expect(project.project_feature.container_registry_access_level).to eq(described_class::ENABLED) + it 'creates project_feature with `enabled` container_registry_access_level' do + expect(project.project_feature.container_registry_access_level).to eq(described_class::ENABLED) + end + end + + context 'when the default is nil' do + it 'creates project_feature with `disabled` container_registry_access_level' do + stub_config_setting(default_projects_features: { container_registry: nil }) + + expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED) + end end end - context 'when the project is created with container_registry_enabled nil' do - it 'creates project with DISABLED container_registry_access_level' do - project = create(:project, container_registry_enabled: nil) + context 'test build factory' do + let(:project) { build(:project, container_registry_access_level: level) } + + subject { project.container_registry_access_level } + + context 'private' do + let(:level) { ProjectFeature::PRIVATE } + + it { is_expected.to eq(level) } + end + + context 'enabled' do + let(:level) { ProjectFeature::ENABLED } + + it { is_expected.to eq(level) } + end + + context 'disabled' do + let(:level) { ProjectFeature::DISABLED } - expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED) + it { is_expected.to eq(level) } end end end diff --git a/spec/models/project_feature_usage_spec.rb b/spec/models/project_feature_usage_spec.rb index 6ef407432b0..698c5374e88 100644 --- a/spec/models/project_feature_usage_spec.rb +++ b/spec/models/project_feature_usage_spec.rb @@ -128,19 +128,15 @@ RSpec.describe ProjectFeatureUsage, type: :model do end context 'ProjectFeatureUsage with DB Load Balancing', :request_store do - include_context 'clear DB Load Balancing configuration' - describe '#log_jira_dvcs_integration_usage' do let!(:project) { create(:project) } subject { project.feature_usage } - context 'database load balancing is configured' do + context 'database load balancing is configured', :db_load_balancing do before do - # Do not pollute AR for other tests, but rather simulate effect of configure_proxy. - allow(ActiveRecord::Base.singleton_class).to receive(:prepend) - ::Gitlab::Database::LoadBalancing.configure_proxy allow(ActiveRecord::Base).to receive(:connection).and_return(::Gitlab::Database::LoadBalancing.proxy) + ::Gitlab::Database::LoadBalancing::Session.clear_session end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index efa269cdb5c..d8f3a63d221 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Project, factory_default: :keep do include ProjectForksHelper include GitHelpers include ExternalAuthorizationServiceHelpers + include ReloadHelpers using RSpec::Parameterized::TableSyntax let_it_be(:namespace) { create_default(:namespace).freeze } @@ -86,7 +87,6 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:ci_pipelines) } it { is_expected.to have_many(:ci_refs) } it { is_expected.to have_many(:builds) } - it { is_expected.to have_many(:build_trace_section_names)} it { is_expected.to have_many(:build_report_results) } it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runners) } @@ -135,6 +135,8 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:pipeline_artifacts) } 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') } + it { is_expected.to have_many(:error_tracking_client_keys).class_name('ErrorTracking::ClientKey') } # GitLab Pages it { is_expected.to have_many(:pages_domains) } @@ -317,7 +319,8 @@ RSpec.describe Project, factory_default: :keep do end it 'validates presence of project_feature' do - project = build(:project, project_feature: nil) + project = build(:project) + project.project_feature = nil expect(project).not_to be_valid end @@ -654,7 +657,6 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) } - it { is_expected.to delegate_method(:allow_editing_commit_messages?).to(:project_setting) } it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) } it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) } @@ -825,8 +827,6 @@ RSpec.describe Project, factory_default: :keep do end describe '#merge_method' do - using RSpec::Parameterized::TableSyntax - where(:ff, :rebase, :method) do true | true | :ff true | false | :ff @@ -1485,33 +1485,21 @@ RSpec.describe Project, factory_default: :keep do end end - describe '.with_active_jira_integrations' do - it 'returns the correct integrations' do - active_jira_integration = create(:jira_integration) - active_service = create(:service, active: true) - - expect(described_class.with_active_jira_integrations).to include(active_jira_integration.project) - expect(described_class.with_active_jira_integrations).not_to include(active_service.project) - end - end - describe '.with_jira_dvcs_cloud' do it 'returns the correct project' do jira_dvcs_cloud_project = create(:project, :jira_dvcs_cloud) - jira_dvcs_server_project = create(:project, :jira_dvcs_server) + create(:project, :jira_dvcs_server) - expect(described_class.with_jira_dvcs_cloud).to include(jira_dvcs_cloud_project) - expect(described_class.with_jira_dvcs_cloud).not_to include(jira_dvcs_server_project) + expect(described_class.with_jira_dvcs_cloud).to contain_exactly(jira_dvcs_cloud_project) end end describe '.with_jira_dvcs_server' do it 'returns the correct project' do jira_dvcs_server_project = create(:project, :jira_dvcs_server) - jira_dvcs_cloud_project = create(:project, :jira_dvcs_cloud) + create(:project, :jira_dvcs_cloud) - expect(described_class.with_jira_dvcs_server).to include(jira_dvcs_server_project) - expect(described_class.with_jira_dvcs_server).not_to include(jira_dvcs_cloud_project) + expect(described_class.with_jira_dvcs_server).to contain_exactly(jira_dvcs_server_project) end end @@ -1597,15 +1585,39 @@ RSpec.describe Project, factory_default: :keep do end describe '.with_integration' do - before do - create_list(:prometheus_project, 2) + it 'returns the correct projects' do + active_confluence_integration = create(:confluence_integration) + inactive_confluence_integration = create(:confluence_integration, active: false) + create(:bugzilla_integration) + + expect(described_class.with_integration(::Integrations::Confluence)).to contain_exactly( + active_confluence_integration.project, + inactive_confluence_integration.project + ) end + end - let(:integration) { :prometheus_integration } + describe '.with_active_integration' do + it 'returns the correct projects' do + active_confluence_integration = create(:confluence_integration) + create(:confluence_integration, active: false) + create(:bugzilla_integration, active: true) - it 'avoids n + 1' do - expect { described_class.with_integration(integration).map(&integration) } - .not_to exceed_query_limit(1) + expect(described_class.with_active_integration(::Integrations::Confluence)).to contain_exactly( + active_confluence_integration.project + ) + end + end + + describe '.include_integration' do + it 'avoids n + 1', :aggregate_failures do + create(:prometheus_integration) + run_test = -> { described_class.include_integration(:prometheus_integration).map(&:prometheus_integration) } + control_count = ActiveRecord::QueryRecorder.new { run_test.call } + create(:prometheus_integration) + + expect(run_test.call.count).to eq(2) + expect { run_test.call }.not_to exceed_query_limit(control_count) end end @@ -1938,8 +1950,6 @@ RSpec.describe Project, factory_default: :keep do end context 'when set to INTERNAL in application settings' do - using RSpec::Parameterized::TableSyntax - before do stub_application_setting(default_project_visibility: Gitlab::VisibilityLevel::INTERNAL) end @@ -2000,8 +2010,6 @@ RSpec.describe Project, factory_default: :keep do end describe '#default_branch_protected?' do - using RSpec::Parameterized::TableSyntax - let_it_be(:namespace) { create(:namespace) } let_it_be(:project) { create(:project, namespace: namespace) } @@ -2405,43 +2413,24 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#set_container_registry_access_level' do + describe '#container_registry_enabled=' do let_it_be_with_reload(:project) { create(:project) } it 'updates project_feature', :aggregate_failures do - # Simulate an existing project that has container_registry enabled - project.update_column(:container_registry_enabled, true) - project.project_feature.update_column(:container_registry_access_level, ProjectFeature::ENABLED) - project.update!(container_registry_enabled: false) - expect(project.read_attribute(:container_registry_enabled)).to eq(false) expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED) project.update!(container_registry_enabled: true) - expect(project.read_attribute(:container_registry_enabled)).to eq(true) expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::ENABLED) end - - it 'rollsback both projects and project_features row in case of error', :aggregate_failures do - project.update_column(:container_registry_enabled, true) - project.project_feature.update_column(:container_registry_access_level, ProjectFeature::ENABLED) - - allow(project).to receive(:valid?).and_return(false) - - expect { project.update!(container_registry_enabled: false) }.to raise_error(ActiveRecord::RecordInvalid) - - expect(project.reload.read_attribute(:container_registry_enabled)).to eq(true) - expect(project.project_feature.reload.container_registry_access_level).to eq(ProjectFeature::ENABLED) - end end describe '#container_registry_enabled' do let_it_be_with_reload(:project) { create(:project) } it 'delegates to project_feature', :aggregate_failures do - project.update_column(:container_registry_enabled, true) project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) expect(project.container_registry_enabled).to eq(false) @@ -2870,6 +2859,36 @@ RSpec.describe Project, factory_default: :keep do it { expect(project.import?).to be true } end + describe '#github_import?' do + let_it_be(:project) { build(:project, import_type: 'github') } + + it { expect(project.github_import?).to be true } + end + + describe '#github_enterprise_import?' do + let_it_be(:github_com_project) do + build( + :project, + import_type: 'github', + import_url: 'https://api.github.com/user/repo' + ) + end + + let_it_be(:github_enterprise_project) do + build( + :project, + import_type: 'github', + import_url: 'https://othergithub.net/user/repo' + ) + end + + it { expect(github_com_project.github_import?).to be true } + it { expect(github_com_project.github_enterprise_import?).to be false } + + it { expect(github_enterprise_project.github_import?).to be true } + it { expect(github_enterprise_project.github_enterprise_import?).to be true } + end + describe '#remove_import_data' do let(:import_data) { ProjectImportData.new(data: { 'test' => 'some data' }) } @@ -2912,10 +2931,6 @@ RSpec.describe Project, factory_default: :keep do subject { project.has_remote_mirror? } - before do - allow_any_instance_of(RemoteMirror).to receive(:refresh_remote) - end - it 'returns true when a remote mirror is enabled' do is_expected.to be_truthy end @@ -2932,10 +2947,6 @@ RSpec.describe Project, factory_default: :keep do delegate :update_remote_mirrors, to: :project - before do - allow_any_instance_of(RemoteMirror).to receive(:refresh_remote) - end - it 'syncs enabled remote mirror' do expect_any_instance_of(RemoteMirror).to receive(:sync) @@ -3011,29 +3022,106 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#ancestors_upto' do - let_it_be(:parent) { create(:group) } - let_it_be(:child) { create(:group, parent: parent) } - let_it_be(:child2) { create(:group, parent: child) } - let_it_be(:project) { create(:project, namespace: child2) } + shared_context 'project with group ancestry' do + let(:parent) { create(:group) } + let(:child) { create(:group, parent: parent) } + let(:child2) { create(:group, parent: child) } + let(:project) { create(:project, namespace: child2) } - it 'returns all ancestors when no namespace is given' do - expect(project.ancestors_upto).to contain_exactly(child2, child, parent) + before do + reload_models(parent, child, child2) end + end + + shared_context 'project with namespace ancestry' do + let(:namespace) { create :namespace } + let(:project) { create :project, namespace: namespace } + end + + shared_examples 'project with group ancestors' do + it 'returns all ancestors' do + is_expected.to contain_exactly(child2, child, parent) + end + end - it 'includes ancestors upto but excluding the given ancestor' do - expect(project.ancestors_upto(parent)).to contain_exactly(child2, child) + shared_examples 'project with ordered group ancestors' do + let(:hierarchy_order) { :desc } + + it 'returns ancestors ordered by descending hierarchy' do + is_expected.to eq([parent, child, child2]) end + end - describe 'with hierarchy_order' do - it 'returns ancestors ordered by descending hierarchy' do - expect(project.ancestors_upto(hierarchy_order: :desc)).to eq([parent, child, child2]) + shared_examples '#ancestors' do + context 'group ancestory' do + include_context 'project with group ancestry' + + it_behaves_like 'project with group ancestors' do + subject { project.ancestors } end - it 'can be used with upto option' do - expect(project.ancestors_upto(parent, hierarchy_order: :desc)).to eq([child, child2]) + it_behaves_like 'project with ordered group ancestors' do + subject { project.ancestors(hierarchy_order: hierarchy_order) } end end + + context 'namespace ancestry' do + include_context 'project with namespace ancestry' + + subject { project.ancestors } + + it { is_expected.to be_empty } + end + end + + describe '#ancestors' do + context 'with linear_project_ancestors feature flag enabled' do + before do + stub_feature_flags(linear_project_ancestors: true) + end + + include_examples '#ancestors' + end + + context 'with linear_project_ancestors feature flag disabled' do + before do + stub_feature_flags(linear_project_ancestors: false) + end + + include_examples '#ancestors' + end + end + + describe '#ancestors_upto' do + context 'group ancestry' do + include_context 'project with group ancestry' + + it_behaves_like 'project with group ancestors' do + subject { project.ancestors_upto } + end + + it_behaves_like 'project with ordered group ancestors' do + subject { project.ancestors_upto(hierarchy_order: hierarchy_order) } + end + + it 'includes ancestors upto but excluding the given ancestor' do + expect(project.ancestors_upto(parent)).to contain_exactly(child2, child) + end + + describe 'with hierarchy_order' do + it 'can be used with upto option' do + expect(project.ancestors_upto(parent, hierarchy_order: :desc)).to eq([child, child2]) + end + end + end + + context 'namespace ancestry' do + include_context 'project with namespace ancestry' + + subject { project.ancestors_upto } + + it { is_expected.to be_empty } + end end describe '#root_ancestor' do @@ -5194,11 +5282,26 @@ RSpec.describe Project, factory_default: :keep do expect(InternalId).to receive(:flush_records!).with(project: project) expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:repository_size]) expect(DetectRepositoryLanguagesWorker).to receive(:perform_async).with(project.id) - expect(project).to receive(:write_repository_config) + expect(AuthorizedProjectUpdate::ProjectRecalculateWorker).to receive(:perform_async).with(project.id) + expect(project).to receive(:set_full_path) project.after_import end + context 'project authorizations refresh' do + it 'updates user authorizations' do + create(:import_state, :started, project: project) + + member = build(:project_member, project: project) + member.importing = true + member.save! + + Sidekiq::Testing.inline! { project.after_import } + + expect(member.user.authorized_project?(project)).to be true + end + end + context 'branch protection' do let_it_be(:namespace) { create(:namespace) } @@ -5263,25 +5366,25 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#write_repository_config' do + describe '#set_full_path' do let_it_be(:project) { create(:project, :repository) } it 'writes full path in .git/config when key is missing' do - project.write_repository_config + project.set_full_path expect(rugged_config['gitlab.fullpath']).to eq project.full_path end it 'updates full path in .git/config when key is present' do - project.write_repository_config(gl_full_path: 'old/path') + project.set_full_path(gl_full_path: 'old/path') - expect { project.write_repository_config }.to change { rugged_config['gitlab.fullpath'] }.from('old/path').to(project.full_path) + expect { project.set_full_path }.to change { rugged_config['gitlab.fullpath'] }.from('old/path').to(project.full_path) end it 'does not raise an error with an empty repository' do project = create(:project_empty_repo) - expect { project.write_repository_config }.not_to raise_error + expect { project.set_full_path }.not_to raise_error end end @@ -5911,10 +6014,9 @@ RSpec.describe Project, factory_default: :keep do end end - context 'with an instance-level and template integrations' do + context 'with an instance-level integration' do before do create(:prometheus_integration, :instance, api_url: 'https://prometheus.instance.com/') - create(:prometheus_integration, :template, api_url: 'https://prometheus.template.com/') end it 'builds the integration from the instance integration' do @@ -5922,17 +6024,7 @@ RSpec.describe Project, factory_default: :keep do end end - context 'with a template integration and no instance-level' do - before do - create(:prometheus_integration, :template, api_url: 'https://prometheus.template.com/') - end - - it 'builds the integration from the template' do - expect(subject.find_or_initialize_integration('prometheus').api_url).to eq('https://prometheus.template.com/') - end - end - - context 'without an exisiting integration, or instance-level or template' do + context 'without an existing integration or instance-level' do it 'builds the integration' do expect(subject.find_or_initialize_integration('prometheus')).to be_a(::Integrations::Prometheus) expect(subject.find_or_initialize_integration('prometheus').api_url).to be_nil @@ -6834,28 +6926,46 @@ RSpec.describe Project, factory_default: :keep do end describe '#package_already_taken?' do - let(:namespace) { create(:namespace) } - let(:project) { create(:project, :public, namespace: namespace) } - let!(:package) { create(:npm_package, project: project, name: "@#{namespace.path}/foo") } + let_it_be(:namespace) { create(:namespace, path: 'test') } + let_it_be(:project) { create(:project, :public, namespace: namespace) } + let_it_be(:package) { create(:npm_package, project: project, name: "@#{namespace.path}/foo", version: '1.2.3') } - context 'no package exists with the same name' do - it 'returns false' do - result = project.package_already_taken?("@#{namespace.path}/bar") - expect(result).to be false + subject { project.package_already_taken?(package_name, package_version, package_type: :npm) } + + context 'within the package project' do + where(:package_name, :package_version, :expected_result) do + '@test/bar' | '1.2.3' | false + '@test/bar' | '5.5.5' | false + '@test/foo' | '1.2.3' | false + '@test/foo' | '5.5.5' | false end - it 'returns false if it is the project that the package belongs to' do - result = project.package_already_taken?("@#{namespace.path}/foo") - expect(result).to be false + with_them do + it { is_expected.to eq expected_result} end end - context 'a package already exists with the same name' do - let(:alt_project) { create(:project, :public, namespace: namespace) } + context 'within a different project' do + let_it_be(:alt_project) { create(:project, :public, namespace: namespace) } - it 'returns true' do - result = alt_project.package_already_taken?("@#{namespace.path}/foo") - expect(result).to be true + subject { alt_project.package_already_taken?(package_name, package_version, package_type: :npm) } + + where(:package_name, :package_version, :expected_result) do + '@test/bar' | '1.2.3' | false + '@test/bar' | '5.5.5' | false + '@test/foo' | '1.2.3' | true + '@test/foo' | '5.5.5' | false + end + + with_them do + it { is_expected.to eq expected_result} + end + + context 'for a different package type' do + it 'returns false' do + result = alt_project.package_already_taken?(package.name, package.version, package_type: :nuget) + expect(result).to be false + end end end end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index cb1baa02e96..ba769e830fd 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -401,12 +401,6 @@ RSpec.describe ProjectStatistics do let(:stat) { :build_artifacts_size } it_behaves_like 'a statistic that increases storage_size asynchronously' - - it_behaves_like 'a statistic that increases storage_size' do - before do - stub_feature_flags(efficient_counter_attribute: false) - end - end end context 'when adjusting :pipeline_artifacts_size' do diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index ce75e68de32..8eab50abd8c 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -193,6 +193,36 @@ RSpec.describe ProjectTeam do end end + describe '#members_with_access_levels' 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(:access_levels) { [Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER] } + + subject(:members_with_access_levels) { project.team.members_with_access_levels(access_levels) } + + before do + project.team.add_developer(developer) + project.team.add_maintainer(maintainer) + project.team.add_guest(guest) + end + + context 'with access_levels' do + it 'filters members who have given access levels' do + expect(members_with_access_levels).to contain_exactly(developer, maintainer) + end + end + + context 'without access_levels' do + let_it_be(:access_levels) { [] } + + it 'returns empty array' do + expect(members_with_access_levels).to be_empty + end + end + end + describe '#add_users' do let(:user1) { create(:user) } let(:user2) { create(:user) } @@ -307,7 +337,7 @@ RSpec.describe ProjectTeam do it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } - context 'but share_with_group_lock is true' do + context 'but share_with_group_lock is true', :sidekiq_inline do before do project.namespace.update!(share_with_group_lock: true) end diff --git a/spec/models/projects/ci_feature_usage_spec.rb b/spec/models/projects/ci_feature_usage_spec.rb new file mode 100644 index 00000000000..674faa6e7d1 --- /dev/null +++ b/spec/models/projects/ci_feature_usage_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::CiFeatureUsage, 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) } + it { is_expected.to validate_presence_of(:feature) } + end + + describe '.insert_usage' do + let_it_be(:project) { create(:project) } + + context 'when data is not a duplicate' do + it 'creates a new record' do + expect { described_class.insert_usage(project_id: project.id, default_branch: false, feature: :code_coverage) } + .to change { described_class.count } + + expect(described_class.first).to have_attributes( + project_id: project.id, + default_branch: false, + feature: 'code_coverage' + ) + end + end + + context 'when data is a duplicate' do + before do + create(:project_ci_feature_usage, project: project, default_branch: false, feature: :code_coverage) + end + + it 'does not create a new record' do + expect { described_class.insert_usage(project_id: project.id, default_branch: false, feature: :code_coverage) } + .not_to change { described_class.count } + end + end + end +end diff --git a/spec/models/release_highlight_spec.rb b/spec/models/release_highlight_spec.rb index a5441e2f47b..14a43df4229 100644 --- a/spec/models/release_highlight_spec.rb +++ b/spec/models/release_highlight_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do - let(:fixture_dir_glob) { Dir.glob(File.join('spec', 'fixtures', 'whats_new', '*.yml')).grep(/\d*\_(\d*\_\d*)\.yml$/) } + let(:fixture_dir_glob) { Dir.glob(File.join(Rails.root, 'spec', 'fixtures', 'whats_new', '*.yml')).grep(/\d*\_(\d*\_\d*)\.yml$/) } before do allow(Dir).to receive(:glob).with(Rails.root.join('data', 'whats_new', '*.yml')).and_return(fixture_dir_glob) @@ -193,4 +193,12 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do expect(subject).to eq('Free') end end + + describe '.file_paths' do + it 'joins relative file paths with the root path to avoid caching the root url' do + allow(described_class).to receive(:relative_file_paths).and_return([+'/a.yml']) + + expect(described_class.file_paths.first).to eq("#{Rails.root}/a.yml") + end + end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index a64b01967ef..382359ccb17 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -93,51 +93,14 @@ RSpec.describe RemoteMirror, :mailer do expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' }) end - it 'updates the remote config if credentials changed' do + it 'does not update the repository config if credentials changed' do mirror = create_mirror(url: 'http://foo:bar@test.com') repo = mirror.project.repository + old_config = rugged_repo(repo).config mirror.update_attribute(:url, 'http://foo:baz@test.com') - config = rugged_repo(repo).config - expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com') - end - - it 'removes previous remote' do - mirror = create_mirror(url: 'http://foo:bar@test.com') - - expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original - - mirror.update(url: 'http://test.com') - end - end - end - - describe '#remote_name' do - context 'when remote name is persisted in the database' do - it 'returns remote name with random value' do - allow(SecureRandom).to receive(:hex).and_return('secret') - - remote_mirror = create(:remote_mirror) - - expect(remote_mirror.remote_name).to eq('remote_mirror_secret') - end - end - - context 'when remote name is not persisted in the database' do - it 'returns remote name with remote mirror id' do - remote_mirror = create(:remote_mirror) - remote_mirror.remote_name = nil - - expect(remote_mirror.remote_name).to eq("remote_mirror_#{remote_mirror.id}") - end - end - - context 'when remote is not persisted in the database' do - it 'returns nil' do - remote_mirror = build(:remote_mirror, remote_name: nil) - - expect(remote_mirror.remote_name).to be_nil + expect(rugged_repo(repo).config.to_hash).to eq(old_config.to_hash) end end end @@ -157,34 +120,19 @@ RSpec.describe RemoteMirror, :mailer do end describe '#update_repository' do - shared_examples 'an update' do - it 'performs update including options' do - git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy) - mirror = build(:remote_mirror) - - expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true) - mirror.update_repository(inmemory_remote: inmemory) - - expect(git_remote_mirror).to have_received(:new).with( - mirror.project.repository.raw, - mirror.remote_name, - inmemory ? mirror.url : nil, - keep_divergent_refs: true - ) - expect(git_remote_mirror).to have_received(:update) - end - end + it 'performs update including options' do + git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy) + mirror = build(:remote_mirror) - context 'with inmemory remote' do - let(:inmemory) { true } + expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true) + mirror.update_repository - it_behaves_like 'an update' - end - - context 'with on-disk remote' do - let(:inmemory) { false } - - it_behaves_like 'an update' + expect(git_remote_mirror).to have_received(:new).with( + mirror.project.repository.raw, + mirror.url, + keep_divergent_refs: true + ) + expect(git_remote_mirror).to have_received(:update) end end @@ -303,10 +251,10 @@ RSpec.describe RemoteMirror, :mailer do end context 'when remote mirror gets destroyed' do - it 'removes remote' do + it 'does not remove the remote' do mirror = create_mirror(url: 'http://foo:bar@test.com') - expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original + expect(RepositoryRemoveRemoteWorker).not_to receive(:perform_async) mirror.destroy! end @@ -402,30 +350,6 @@ RSpec.describe RemoteMirror, :mailer do end end - describe '#ensure_remote!' do - let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } - let(:project) { remote_mirror.project } - let(:repository) { project.repository } - - it 'adds a remote multiple times with no errors' do - expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, remote_mirror.url).twice.and_call_original - - 2.times do - remote_mirror.ensure_remote! - end - end - - context 'SSH public-key authentication' do - it 'omits the password from the URL' do - remote_mirror.update!(auth_method: 'ssh_public_key', url: 'ssh://git:pass@example.com') - - expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, 'ssh://git@example.com') - - remote_mirror.ensure_remote! - end - end - end - describe '#url=' do let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 452eafe733f..211e448b6cf 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -398,32 +398,47 @@ RSpec.describe Repository do end describe '#new_commits' do - let_it_be(:project) { create(:project, :repository) } + shared_examples '#new_commits' do + let_it_be(:project) { create(:project, :repository) } - let(:repository) { project.repository } + let(:repository) { project.repository } - subject { repository.new_commits(rev) } + subject { repository.new_commits(rev, allow_quarantine: allow_quarantine) } - context 'when there are no new commits' do - let(:rev) { repository.commit.id } + context 'when there are no new commits' do + let(:rev) { repository.commit.id } - it 'returns an empty array' do - expect(subject).to eq([]) + it 'returns an empty array' do + expect(subject).to eq([]) + end end - end - context 'when new commits are found' do - let(:branch) { 'orphaned-branch' } - let!(:rev) { repository.commit(branch).id } + context 'when new commits are found' do + let(:branch) { 'orphaned-branch' } + let!(:rev) { repository.commit(branch).id } + let(:allow_quarantine) { false } - it 'returns the commits' do - repository.delete_branch(branch) + it 'returns the commits' do + repository.delete_branch(branch) - expect(subject).not_to be_empty - expect(subject).to all( be_a(::Commit) ) - expect(subject.size).to eq(1) + expect(subject).not_to be_empty + expect(subject).to all( be_a(::Commit) ) + expect(subject.size).to eq(1) + end end end + + context 'with quarantine' do + let(:allow_quarantine) { true } + + it_behaves_like '#new_commits' + end + + context 'without quarantine' do + let(:allow_quarantine) { false } + + it_behaves_like '#new_commits' + end end describe '#commits_by' do @@ -1094,99 +1109,16 @@ RSpec.describe Repository do end end - describe '#async_remove_remote' do - before do - masterrev = repository.find_branch('master').dereferenced_target - create_remote_branch('joe', 'remote_branch', masterrev) - end - - context 'when worker is scheduled successfully' do - before do - masterrev = repository.find_branch('master').dereferenced_target - create_remote_branch('remote_name', 'remote_branch', masterrev) - - allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return('1234') - end - - it 'returns job_id' do - expect(repository.async_remove_remote('joe')).to eq('1234') - end - end - - context 'when worker does not schedule successfully' do - before do - allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return(nil) - end - - it 'returns nil' do - expect(Gitlab::AppLogger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.") - - expect(repository.async_remove_remote('joe')).to be_nil - end - end - end - describe '#fetch_as_mirror' do let(:url) { "http://example.com" } - context 'when :fetch_remote_params is enabled' do - let(:remote_name) { "remote-name" } - - before do - stub_feature_flags(fetch_remote_params: true) - end - - it 'fetches the URL without creating a remote' do - expect(repository).not_to receive(:add_remote) - expect(repository) - .to receive(:fetch_remote) - .with(remote_name, url: url, forced: false, prune: true, refmap: :all_refs) - .and_return(nil) - - repository.fetch_as_mirror(url, remote_name: remote_name) - end - end - - context 'when :fetch_remote_params is disabled' do - before do - stub_feature_flags(fetch_remote_params: false) - end - - shared_examples 'a fetch' do - it 'adds and fetches a remote' do - expect(repository) - .to receive(:add_remote) - .with(expected_remote, url, mirror_refmap: :all_refs) - .and_return(nil) - expect(repository) - .to receive(:fetch_remote) - .with(expected_remote, forced: false, prune: true) - .and_return(nil) - - repository.fetch_as_mirror(url, remote_name: remote_name) - end - end - - context 'with temporary remote' do - let(:remote_name) { nil } - let(:expected_remote_suffix) { "123456" } - let(:expected_remote) { "tmp-#{expected_remote_suffix}" } - - before do - expect(repository) - .to receive(:async_remove_remote).with(expected_remote).and_return(nil) - allow(SecureRandom).to receive(:hex).and_return(expected_remote_suffix) - end - - it_behaves_like 'a fetch' - end - - context 'with remote name' do - let(:remote_name) { "foo" } - let(:expected_remote) { "foo" } + it 'fetches the URL without creating a remote' do + expect(repository) + .to receive(:fetch_remote) + .with(url, forced: false, prune: true, refmap: :all_refs, http_authorization_header: "") + .and_return(nil) - it_behaves_like 'a fetch' - end + repository.fetch_as_mirror(url) end end @@ -2605,24 +2537,46 @@ RSpec.describe Repository do end shared_examples '#tree' do + subject { repository.tree(sha, path, recursive: recursive, pagination_params: pagination_params) } + + let(:sha) { :head } + let(:path) { nil } + let(:recursive) { false } + let(:pagination_params) { nil } + context 'using a non-existing repository' do before do allow(repository).to receive(:head_commit).and_return(nil) end - it 'returns nil' do - expect(repository.tree(:head)).to be_nil - end + it { is_expected.to be_nil } + + context 'when path is defined' do + let(:path) { 'README.md' } - it 'returns nil when using a path' do - expect(repository.tree(:head, 'README.md')).to be_nil + it { is_expected.to be_nil } end end context 'using an existing repository' do - it 'returns a Tree' do - expect(repository.tree(:head)).to be_an_instance_of(Tree) - expect(repository.tree('v1.1.1')).to be_an_instance_of(Tree) + it { is_expected.to be_an_instance_of(Tree) } + + context 'when different sha is set' do + let(:sha) { 'v1.1.1' } + + it { is_expected.to be_an_instance_of(Tree) } + end + + context 'when recursive is true' do + let(:recursive) { true } + + it { is_expected.to be_an_instance_of(Tree) } + end + + context 'with pagination parameters' do + let(:pagination_params) { { limit: 10, page_token: nil } } + + it { is_expected.to be_an_instance_of(Tree) } end end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 19d3895177f..4e20a83f18e 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -431,7 +431,7 @@ RSpec.describe Snippet do subject do snippet.assign_attributes(title: title) - snippet.check_for_spam? + snippet.check_for_spam?(user: snippet.author) end context 'when public and spammable attributes changed' do @@ -455,7 +455,7 @@ RSpec.describe Snippet do snippet.save! snippet.visibility_level = Snippet::PUBLIC - expect(snippet.check_for_spam?).to be_truthy + expect(snippet.check_for_spam?(user: snippet.author)).to be_truthy end end diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index 9d6fda1d2a9..d6c31307e30 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -70,8 +70,9 @@ RSpec.describe Timelog do let_it_be(:medium_time_ago) { 15.days.ago } let_it_be(:long_time_ago) { 65.days.ago } - let_it_be(:timelog) { create(:issue_timelog, spent_at: long_time_ago) } - let_it_be(:timelog1) { create(:issue_timelog, spent_at: medium_time_ago, issue: group_issue) } + let_it_be(:user) { create(:user) } + let_it_be(:timelog) { create(:issue_timelog, spent_at: long_time_ago, user: user) } + let_it_be(:timelog1) { create(:issue_timelog, spent_at: medium_time_ago, issue: group_issue, user: user) } let_it_be(:timelog2) { create(:issue_timelog, spent_at: short_time_ago, issue: subgroup_issue) } let_it_be(:timelog3) { create(:merge_request_timelog, spent_at: long_time_ago) } let_it_be(:timelog4) { create(:merge_request_timelog, spent_at: medium_time_ago, merge_request: group_merge_request) } @@ -83,6 +84,25 @@ RSpec.describe Timelog do end end + describe '.for_user' do + it 'return timelogs created by user' do + expect(described_class.for_user(user)).to contain_exactly(timelog, timelog1) + end + end + + describe '.in_project' do + it 'returns timelogs created for project issues and merge requests' do + project = create(:project, :empty_repo) + + create(:issue_timelog) + create(:merge_request_timelog) + timelog1 = create(:issue_timelog, issue: create(:issue, project: project)) + timelog2 = create(:merge_request_timelog, merge_request: create(:merge_request, source_project: project)) + + expect(described_class.in_project(project.id)).to contain_exactly(timelog1, timelog2) + end + end + describe '.at_or_after' do it 'returns timelogs at the time limit' do timelogs = described_class.at_or_after(short_time_ago) diff --git a/spec/models/tree_spec.rb b/spec/models/tree_spec.rb index 1522d836f76..b7a8276ec55 100644 --- a/spec/models/tree_spec.rb +++ b/spec/models/tree_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Tree do let(:repository) { create(:project, :repository).repository } let(:sha) { repository.root_ref } - subject { described_class.new(repository, '54fcc214') } + subject(:tree) { described_class.new(repository, '54fcc214') } describe '#readme' do before do @@ -66,4 +66,10 @@ RSpec.describe Tree do expect(subject.readme.name).to eq 'README.md' end end + + describe '#cursor' do + subject { tree.cursor } + + it { is_expected.to be_an_instance_of(Gitaly::PaginationCursor) } + end end diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index c2d9b916a1c..3c87dcdcbd9 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -16,6 +16,11 @@ RSpec.describe UserDetail do it { is_expected.to validate_length_of(:pronouns).is_at_most(50) } end + describe '#pronunciation' do + it { is_expected.not_to validate_presence_of(:pronunciation) } + it { is_expected.to validate_length_of(:pronunciation).is_at_most(255) } + end + describe '#bio' do it { is_expected.to validate_length_of(:bio).is_at_most(255) } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0eb769c65cd..d73bc95a2f2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -77,6 +77,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:pronouns).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:pronouns=).to(:user_detail).with_arguments(:args).allow_nil } + it { is_expected.to delegate_method(:pronunciation).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:pronunciation=).to(:user_detail).with_arguments(:args).allow_nil } + it { is_expected.to delegate_method(:bio).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:bio=).to(:user_detail).with_arguments(:args).allow_nil } it { is_expected.to delegate_method(:bio_html).to(:user_detail).allow_nil } @@ -89,6 +92,7 @@ RSpec.describe User do it { is_expected.to have_one(:atlassian_identity) } it { is_expected.to have_one(:user_highest_role) } it { is_expected.to have_one(:credit_card_validation) } + it { is_expected.to have_one(:banned_user) } it { is_expected.to have_many(:snippets).dependent(:destroy) } it { is_expected.to have_many(:members) } it { is_expected.to have_many(:project_members) } @@ -120,6 +124,7 @@ RSpec.describe User do it { is_expected.to have_many(:merge_request_reviewers).inverse_of(:reviewer) } it { is_expected.to have_many(:created_custom_emoji).inverse_of(:creator) } it { is_expected.to have_many(:in_product_marketing_emails) } + it { is_expected.to have_many(:timelogs) } describe "#user_detail" do it 'does not persist `user_detail` by default' do @@ -145,6 +150,12 @@ RSpec.describe User do expect(user.pronouns).to eq(user.user_detail.pronouns) end + it 'delegates `pronunciation` to `user_detail`' do + user = create(:user, name: 'Example', pronunciation: 'uhg-zaam-pl') + + expect(user.pronunciation).to eq(user.user_detail.pronunciation) + end + it 'creates `user_detail` when `bio` is first updated' do user = create(:user) @@ -485,7 +496,7 @@ RSpec.describe User do describe 'email' do context 'when no signup domains allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return([]) + stub_application_setting(domain_allowlist: []) end it 'accepts any email' do @@ -496,7 +507,7 @@ RSpec.describe User do context 'bad regex' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['([a-zA-Z0-9]+)+\.com']) + stub_application_setting(domain_allowlist: ['([a-zA-Z0-9]+)+\.com']) end it 'does not hang on evil input' do @@ -510,7 +521,7 @@ RSpec.describe User do context 'when a signup domain is allowed and subdomains are allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['example.com', '*.example.com']) + stub_application_setting(domain_allowlist: ['example.com', '*.example.com']) end it 'accepts info@example.com' do @@ -526,12 +537,13 @@ RSpec.describe User do it 'rejects example@test.com' do user = build(:user, email: "example@test.com") expect(user).to be_invalid + expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) end end context 'when a signup domain is allowed and subdomains are not allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['example.com']) + stub_application_setting(domain_allowlist: ['example.com']) end it 'accepts info@example.com' do @@ -542,11 +554,13 @@ RSpec.describe User do it 'rejects info@test.example.com' do user = build(:user, email: "info@test.example.com") expect(user).to be_invalid + expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) end it 'rejects example@test.com' do user = build(:user, email: "example@test.com") expect(user).to be_invalid + expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) end it 'accepts example@test.com when added by another user' do @@ -557,13 +571,13 @@ RSpec.describe User do context 'domain denylist' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist_enabled?).and_return(true) - allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist).and_return(['example.com']) + stub_application_setting(domain_denylist_enabled: true) + stub_application_setting(domain_denylist: ['example.com']) end context 'bad regex' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist).and_return(['([a-zA-Z0-9]+)+\.com']) + stub_application_setting(domain_denylist: ['([a-zA-Z0-9]+)+\.com']) end it 'does not hang on evil input' do @@ -584,6 +598,7 @@ RSpec.describe User do it 'rejects info@example.com' do user = build(:user, email: 'info@example.com') expect(user).not_to be_valid + expect(user.errors.messages[:email].first).to eq(_('is not from an allowed domain.')) end it 'accepts info@example.com when added by another user' do @@ -594,8 +609,8 @@ RSpec.describe User do context 'when a signup domain is denied but a wildcard subdomain is allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist).and_return(['test.example.com']) - allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['*.example.com']) + stub_application_setting(domain_denylist: ['test.example.com']) + stub_application_setting(domain_allowlist: ['*.example.com']) end it 'gives priority to allowlist and allow info@test.example.com' do @@ -606,7 +621,7 @@ RSpec.describe User do context 'with both lists containing a domain' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['test.com']) + stub_application_setting(domain_allowlist: ['test.com']) end it 'accepts info@test.com' do @@ -617,6 +632,7 @@ RSpec.describe User do it 'rejects info@example.com' do user = build(:user, email: 'info@example.com') expect(user).not_to be_valid + expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.')) end end end @@ -688,7 +704,7 @@ RSpec.describe User do user.notification_email = email.email expect(user).to be_invalid - expect(user.errors[:notification_email]).to include('is not an email you own') + expect(user.errors[:notification_email]).to include(_('must be an email you have verified')) end end @@ -707,7 +723,7 @@ RSpec.describe User do user.public_email = email.email expect(user).to be_invalid - expect(user.errors[:public_email]).to include('is not an email you own') + expect(user.errors[:public_email]).to include(_('must be an email you have verified')) end end @@ -1798,6 +1814,15 @@ RSpec.describe User do it { expect(user.namespaces).to contain_exactly(user.namespace, group) } it { expect(user.manageable_namespaces).to contain_exactly(user.namespace, group) } + context 'with owned groups only' do + before do + other_group = create(:group) + other_group.add_developer(user) + end + + it { expect(user.namespaces(owned_only: true)).to contain_exactly(user.namespace, group) } + end + context 'with child groups' do let!(:subgroup) { create(:group, parent: group) } @@ -1950,6 +1975,42 @@ RSpec.describe User do end end + describe 'banning and unbanning a user', :aggregate_failures do + let(:user) { create(:user) } + + context 'banning a user' do + it 'bans and blocks the user' do + user.ban + + expect(user.banned?).to eq(true) + expect(user.blocked?).to eq(true) + end + + it 'creates a BannedUser record' do + expect { user.ban }.to change { Users::BannedUser.count }.by(1) + expect(Users::BannedUser.last.user_id).to eq(user.id) + end + end + + context 'unbanning a user' do + before do + user.ban! + end + + it 'activates the user' do + user.activate + + expect(user.banned?).to eq(false) + expect(user.active?).to eq(true) + end + + it 'deletes the BannedUser record' do + expect { user.activate }.to change { Users::BannedUser.count }.by(-1) + expect(Users::BannedUser.where(user_id: user.id)).not_to exist + end + end + end + describe '.filter_items' do let(:user) { double } @@ -3064,6 +3125,19 @@ RSpec.describe User do end end + describe '#notification_email' do + let(:email) { 'gonzo@muppets.com' } + + context 'when the column in the database is null' do + subject { create(:user, email: email, notification_email: nil) } + + it 'defaults to the primary email' do + expect(subject.read_attribute(:notification_email)).to be nil + expect(subject.notification_email).to eq(email) + end + end + end + describe '.find_by_private_commit_email' do context 'with email' do let_it_be(:user) { create(:user) } @@ -3993,6 +4067,14 @@ RSpec.describe User do ] end end + + context 'when the user is not saved' do + let(:user) { build(:user) } + + it 'returns empty when there are no groups or ancestor groups for the user' do + is_expected.to eq([]) + end + end end describe '#refresh_authorized_projects', :clean_gitlab_redis_shared_state do @@ -4254,6 +4336,14 @@ RSpec.describe User do expect(user.two_factor_grace_period).to be 48 end end + + context 'when the user is not saved' do + let(:user) { build(:user) } + + it 'does not raise an ActiveRecord::StatementInvalid statement exception' do + expect { user.update_two_factor_requirement }.not_to raise_error + end + end end describe '#source_groups_of_two_factor_authentication_requirement' do diff --git a/spec/models/users/banned_user_spec.rb b/spec/models/users/banned_user_spec.rb new file mode 100644 index 00000000000..b55c4821d05 --- /dev/null +++ b/spec/models/users/banned_user_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::BannedUser do + describe 'relationships' do + it { is_expected.to belong_to(:user) } + end + + describe 'validations' do + before do + create(:user, :banned) + end + + it { is_expected.to validate_presence_of(:user) } + + it 'validates uniqueness of banned user id' do + is_expected.to validate_uniqueness_of(:user_id).with_message("banned user already exists") + end + end +end diff --git a/spec/models/users/in_product_marketing_email_spec.rb b/spec/models/users/in_product_marketing_email_spec.rb index 772d875d69e..a9ddd86677c 100644 --- a/spec/models/users/in_product_marketing_email_spec.rb +++ b/spec/models/users/in_product_marketing_email_spec.rb @@ -19,6 +19,12 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:track, :series]).with_message('has already been sent') } end + describe '.tracks' do + it 'has an entry for every track' do + expect(Namespaces::InProductMarketingEmailsService::TRACKS.keys).to match_array(described_class.tracks.keys.map(&:to_sym)) + end + end + describe '.without_track_and_series' do let_it_be(:user) { create(:user) } diff --git a/spec/models/work_item/type_spec.rb b/spec/models/work_item/type_spec.rb new file mode 100644 index 00000000000..90f551b7d63 --- /dev/null +++ b/spec/models/work_item/type_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItem::Type do + describe 'modules' do + it { is_expected.to include_module(CacheMarkdownField) } + end + + describe 'associations' do + it { is_expected.to have_many(:work_items).with_foreign_key('work_item_type_id') } + it { is_expected.to belong_to(:namespace) } + end + + describe '#destroy' do + let!(:work_item) { create :issue } + + context 'when there are no work items of that type' do + it 'deletes type but not unrelated issues' do + type = create(:work_item_type) + + expect { type.destroy! }.not_to change(Issue, :count) + expect(WorkItem::Type.count).to eq 0 + end + end + + it 'does not delete type when there are related issues' do + type = create(:work_item_type, work_items: [work_item]) + + expect { type.destroy! }.to raise_error(ActiveRecord::InvalidForeignKey) + expect(Issue.count).to eq 1 + end + end + + describe 'validation' do + describe 'name uniqueness' do + subject { create(:work_item_type) } + + it { is_expected.to validate_uniqueness_of(:name).case_insensitive.scoped_to([:namespace_id]) } + end + + it { is_expected.not_to allow_value('s' * 256).for(:icon_name) } + end + + describe '#name' do + it 'strips name' do + work_item_type = described_class.new(name: ' label😸 ') + work_item_type.valid? + + expect(work_item_type.name).to eq('label😸') + end + end +end |