diff options
author | Rubén Dávila Santos <ruben@gitlab.com> | 2016-08-20 15:21:50 +0000 |
---|---|---|
committer | Rubén Dávila Santos <ruben@gitlab.com> | 2016-08-20 15:21:50 +0000 |
commit | 6a05f24621646790d1f5bccd0abc003b5386d0f0 (patch) | |
tree | c5ccf8bd6806f024db38299400517ef056b2be7b | |
parent | a16ac37e91cefec81d53b7ade755b04c67b4912c (diff) | |
parent | 1954bb17eca49d375c92a4b8fa7f52fa39873a7d (diff) | |
download | gitlab-ce-6a05f24621646790d1f5bccd0abc003b5386d0f0.tar.gz |
Merge branch 'fix-rspec-failures-due-to-cached-permissions' into 'master'
Fix assorted rspec failures due to stale, cached user permissions
RequestStore is disabled in tests, but the Ability class was caching user permissions based on the user and project ID of previous test runs. Revise code to use RequestStore only if it is active.
See merge request !5919
-rw-r--r-- | app/models/ability.rb | 48 | ||||
-rw-r--r-- | spec/models/ability_spec.rb | 64 |
2 files changed, 91 insertions, 21 deletions
diff --git a/app/models/ability.rb b/app/models/ability.rb index 07f703f205d..a49dd703926 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -166,38 +166,44 @@ class Ability end def project_abilities(user, project) - rules = [] key = "/user/#{user.id}/project/#{project.id}" - RequestStore.store[key] ||= begin - # Push abilities on the users team role - rules.push(*project_team_rules(project.team, user)) + if RequestStore.active? + RequestStore.store[key] ||= uncached_project_abilities(user, project) + else + uncached_project_abilities(user, project) + end + end - owner = user.admin? || - project.owner == user || - (project.group && project.group.has_owner?(user)) + def uncached_project_abilities(user, project) + rules = [] + # Push abilities on the users team role + rules.push(*project_team_rules(project.team, user)) - if owner - rules.push(*project_owner_rules) - end + owner = user.admin? || + project.owner == user || + (project.group && project.group.has_owner?(user)) - if project.public? || (project.internal? && !user.external?) - rules.push(*public_project_rules) + if owner + rules.push(*project_owner_rules) + end - # Allow to read builds for internal projects - rules << :read_build if project.public_builds? + if project.public? || (project.internal? && !user.external?) + rules.push(*public_project_rules) - unless owner || project.team.member?(user) || project_group_member?(project, user) - rules << :request_access if project.request_access_enabled - end - end + # Allow to read builds for internal projects + rules << :read_build if project.public_builds? - if project.archived? - rules -= project_archived_rules + unless owner || project.team.member?(user) || project_group_member?(project, user) + rules << :request_access if project.request_access_enabled end + end - rules - project_disabled_features_rules(project) + if project.archived? + rules -= project_archived_rules end + + (rules - project_disabled_features_rules(project)).uniq end def project_team_rules(team, user) diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 853f6943cef..aa3b2bbf471 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -171,6 +171,70 @@ describe Ability, lib: true do end end + shared_examples_for ".project_abilities" do |enable_request_store| + before do + RequestStore.begin! if enable_request_store + end + + after do + if enable_request_store + RequestStore.end! + RequestStore.clear! + end + end + + describe '.project_abilities' do + let!(:project) { create(:empty_project, :public) } + let!(:user) { create(:user) } + + it 'returns permissions for admin user' do + admin = create(:admin) + + results = described_class.project_abilities(admin, project) + + expect(results.count).to eq(68) + end + + it 'returns permissions for an owner' do + results = described_class.project_abilities(project.owner, project) + + expect(results.count).to eq(68) + end + + it 'returns permissions for a master' do + project.team << [user, :master] + + results = described_class.project_abilities(user, project) + + expect(results.count).to eq(60) + end + + it 'returns permissions for a developer' do + project.team << [user, :developer] + + results = described_class.project_abilities(user, project) + + expect(results.count).to eq(44) + end + + it 'returns permissions for a guest' do + project.team << [user, :guest] + + results = described_class.project_abilities(user, project) + + expect(results.count).to eq(21) + end + end + end + + describe '.project_abilities with RequestStore' do + it_behaves_like ".project_abilities", true + end + + describe '.project_abilities without RequestStore' do + it_behaves_like ".project_abilities", false + end + describe '.issues_readable_by_user' do context 'with an admin user' do it 'returns all given issues' do |