diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-03-06 23:30:47 +0100 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-03-07 16:59:17 +0100 |
commit | 9aabd8fd5ecb4090515db48692f3d064624aec0a (patch) | |
tree | 318418b94d97bc846e6e00ed782181cfd26e9487 /app | |
parent | 9b27027619580bffeffa88965007c2c29ac9648c (diff) | |
download | gitlab-ce-9aabd8fd5ecb4090515db48692f3d064624aec0a.tar.gz |
Limit queries to a user-branch combination
The query becomes a lot simpler if we can check the branch name as
well instead of having to load all branch names.
Diffstat (limited to 'app')
-rw-r--r-- | app/models/project.rb | 45 | ||||
-rw-r--r-- | app/policies/project_policy.rb | 18 |
2 files changed, 32 insertions, 31 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 0128967e038..a2b2e0554b1 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1800,12 +1800,31 @@ class Project < ActiveRecord::Base Badge.where("id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end - def branches_allowing_maintainer_access_to_user(user) - @branches_allowing_maintainer_access_to_user ||= Hash.new do |result, user| - result[user] = fetch_branches_allowing_maintainer_access_to_user(user) + def merge_requests_allowing_push_to_user(user) + return MergeRequest.none unless user + + developer_access_exists = user.project_authorizations + .where('access_level >= ? ', Gitlab::Access::DEVELOPER) + .where('project_authorizations.project_id = merge_requests.target_project_id') + .limit(1) + .select(1) + source_of_merge_requests.opened + .where(allow_maintainer_to_push: true) + .where('EXISTS (?)', developer_access_exists) + end + + def branch_allows_maintainer_push?(user, branch_name) + return false unless user + + cache_key = "user:#{user.id}:#{branch_name}:branch_allows_push" + + memoized_results = strong_memoize(:branch_allows_maintainer_push) do + Hash.new do |result, cache_key| + result[cache_key] = fetch_branch_allows_maintainer_push?(user, branch_name) + end end - @branches_allowing_maintainer_access_to_user[user] + memoized_results[cache_key] end private @@ -1931,23 +1950,13 @@ class Project < ActiveRecord::Base raise ex end - def fetch_branches_allowing_maintainer_access_to_user(user) - return [] unless user - - projects_with_developer_access = user.project_authorizations - .where('access_level >= ? ', Gitlab::Access::DEVELOPER) - .select(:project_id) - merge_requests_allowing_push = source_of_merge_requests.opened - .where(allow_maintainer_to_push: true) - .where(target_project_id: projects_with_developer_access) - .select(:source_branch).uniq - + def fetch_branch_allows_maintainer_push?(user, branch_name) if RequestStore.active? - RequestStore.fetch("project-#{id}:user-#{user.id}:branches_allowing_maintainer_access") do - merge_requests_allowing_push.pluck(:source_branch) + RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_maintainer_push") do + merge_requests_allowing_push_to_user(user).where(source_branch: branch_name).any? end else - merge_requests_allowing_push.pluck(:source_branch) + merge_requests_allowing_push_to_user(user).where(source_branch: branch_name).any? end end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index e143a32e350..57ab0c23dcd 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -61,9 +61,9 @@ class ProjectPolicy < BasePolicy desc "Project has request access enabled" condition(:request_access_enabled, scope: :subject) { project.request_access_enabled } - desc "The project has merge requests open that allow external users to push" - condition(:merge_request_allows_push, scope: :subject) do - project.branches_allowing_maintainer_access_to_user(@user).any? + desc "Has merge requests allowing pushes to user" + condition(:has_merge_requests_allowing_pushes, scope: :subject) do + project.merge_requests_allowing_push_to_user(user).any? end features = %w[ @@ -245,7 +245,6 @@ class ProjectPolicy < BasePolicy rule { repository_disabled }.policy do prevent :push_code - prevent :push_single_branch prevent :download_code prevent :fork_project prevent :read_commit_status @@ -298,21 +297,14 @@ class ProjectPolicy < BasePolicy end # These rules are included to allow maintainers of projects to push to certain - # branches of forks. - rule { can?(:public_access) & merge_request_allows_push }.policy do - enable :push_single_branch + # to run pipelines for the branches they have access to. + rule { can?(:public_access) & has_merge_requests_allowing_pushes }.policy do enable :create_build enable :update_build enable :create_pipeline enable :update_pipeline end - # A wrapper around `push_code` and `push_single_branch` to avoid several - # `push_code`: User can push everything to the repo - # `push_single_brach`: User can push to a single branch in the repo - # `push_to_repo`: User can push something to this repo. - rule { can?(:push_code) | can?(:push_single_branch) }.enable :push_to_repo - private def team_member? |