diff options
author | Sean McGivern <sean@gitlab.com> | 2019-01-04 08:42:20 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-01-04 08:42:20 +0000 |
commit | 626f3d03676cb7d714d9c5e9d78c76bdfb2ddda0 (patch) | |
tree | b765d63b06e18561e463ef77d3ceb04146a26c00 /spec | |
parent | d56124b5e16e15afd830a1bdc8c34a4a57d898d8 (diff) | |
parent | e893f560e01c0039a4fc4c6c93be8e82b5888b3b (diff) | |
download | gitlab-ce-626f3d03676cb7d714d9c5e9d78c76bdfb2ddda0.tar.gz |
Merge branch 'dm-git-access-any-ce' into 'master'
[CE] Don't run checks for changed refs when specific changes are unknown
See merge request gitlab-org/gitlab-ce!23990
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/checks/push_check_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 18 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_wiki_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 36 | ||||
-rw-r--r-- | spec/serializers/pipeline_serializer_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/ci/retry_pipeline_service_spec.rb | 2 |
6 files changed, 40 insertions, 22 deletions
diff --git a/spec/lib/gitlab/checks/push_check_spec.rb b/spec/lib/gitlab/checks/push_check_spec.rb index 25f0d428cb9..e1bd52d6c0b 100644 --- a/spec/lib/gitlab/checks/push_check_spec.rb +++ b/spec/lib/gitlab/checks/push_check_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Checks::PushCheck do context 'when the user is not allowed to push to the repo' do it 'raises an error' do expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) - expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false) + expect(project).to receive(:branch_allows_collaboration?).with(user_access.user, 'master').and_return(false) expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 8d8eb50ad76..3e34dd592f2 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::GitAccess do let(:authentication_abilities) { %i[read_project download_code push_code] } let(:redirected_path) { nil } let(:auth_result_type) { nil } - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } let(:push_access_check) { access.check('git-receive-pack', changes) } let(:pull_access_check) { access.check('git-upload-pack', changes) } @@ -437,7 +437,7 @@ describe Gitlab::GitAccess do let(:project) { nil } context 'when changes is _any' do - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } context 'when authentication abilities include push code' do let(:authentication_abilities) { [:push_code] } @@ -483,7 +483,7 @@ describe Gitlab::GitAccess do end context 'when project exists' do - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } let!(:project) { create(:project) } it 'does not create a new project' do @@ -497,7 +497,7 @@ describe Gitlab::GitAccess do let(:project_path) { "nonexistent" } let(:project) { nil } let(:namespace_path) { user.namespace.path } - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } it 'does not create a new project' do expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } @@ -507,7 +507,7 @@ describe Gitlab::GitAccess do context 'when pull' do let(:cmd) { 'git-upload-pack' } - let(:changes) { '_any' } + let(:changes) { Gitlab::GitAccess::ANY } context 'when project does not exist' do let(:project_path) { "new-project" } @@ -736,7 +736,8 @@ describe Gitlab::GitAccess do end let(:changes) do - { push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", + { any: Gitlab::GitAccess::ANY, + push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", push_master: '6f6d7e7ed 570e7b2ab refs/heads/master', push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature', push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\ @@ -798,6 +799,7 @@ describe Gitlab::GitAccess do permissions_matrix = { admin: { + any: true, push_new_branch: true, push_master: true, push_protected_branch: true, @@ -809,6 +811,7 @@ describe Gitlab::GitAccess do }, maintainer: { + any: true, push_new_branch: true, push_master: true, push_protected_branch: true, @@ -820,6 +823,7 @@ describe Gitlab::GitAccess do }, developer: { + any: true, push_new_branch: true, push_master: true, push_protected_branch: false, @@ -831,6 +835,7 @@ describe Gitlab::GitAccess do }, reporter: { + any: false, push_new_branch: false, push_master: false, push_protected_branch: false, @@ -842,6 +847,7 @@ describe Gitlab::GitAccess do }, guest: { + any: false, push_new_branch: false, push_master: false, push_protected_branch: false, diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 9c6c9fe13bf..6ba65b56618 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -38,7 +38,7 @@ describe Gitlab::GitAccessWiki do end describe '#access_check_download!' do - subject { access.check('git-upload-pack', '_any') } + subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) } before do project.add_developer(user) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4b6592020c1..fa0f3620806 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3831,6 +3831,16 @@ describe Project do let(:user) { create(:user) } let(:target_project) { create(:project, :repository) } let(:project) { fork_project(target_project, nil, repository: true) } + let!(:local_merge_request) do + create( + :merge_request, + target_project: project, + target_branch: 'target-branch', + source_project: project, + source_branch: 'awesome-feature-1', + allow_collaboration: true + ) + end let!(:merge_request) do create( :merge_request, @@ -3875,14 +3885,23 @@ describe Project do end end - describe '#branch_allows_collaboration_push?' do - it 'allows access if the user can merge the merge request' do - expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) + describe '#any_branch_allows_collaboration?' do + it 'allows access when there are merge requests open allowing collaboration' do + expect(project.any_branch_allows_collaboration?(user)) .to be_truthy end - it 'allows access when there are merge requests open but no branch name is given' do - expect(project.branch_allows_collaboration?(user, nil)) + it 'does not allow access when there are no merge requests open allowing collaboration' do + merge_request.close! + + expect(project.any_branch_allows_collaboration?(user)) + .to be_falsey + end + end + + describe '#branch_allows_collaboration?' do + it 'allows access if the user can merge the merge request' do + expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) .to be_truthy end @@ -3913,13 +3932,6 @@ describe Project do .to be_falsy end - it 'caches the result' do - control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } - - expect { 3.times { project.branch_allows_collaboration?(user, 'awesome-feature-1') } } - .not_to exceed_query_limit(control) - end - context 'when the requeststore is active', :request_store do it 'only queries per project across instances' do control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index cf57776346a..79aa32b29bb 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -144,7 +144,7 @@ describe PipelineSerializer do # pipeline. With the same ref this check is cached but if refs are # different then there is an extra query per ref # https://gitlab.com/gitlab-org/gitlab-ce/issues/46368 - expect(recorded.count).to be_within(2).of(34) + expect(recorded.count).to be_within(2).of(38) expect(recorded.cached_count).to eq(0) end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 55445e71539..75042b29bea 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -285,7 +285,7 @@ describe Ci::RetryPipelineService, '#execute' do end it 'allows to retry failed pipeline' do - allow_any_instance_of(Project).to receive(:fetch_branch_allows_collaboration?).and_return(true) + allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true) allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) service.execute(pipeline) |