From 9b2ad62a069961391ea7492a51eaad88ad2fce46 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Tue, 9 Aug 2016 09:38:48 +0200 Subject: Load project members, group members in bulk and cache it --- app/finders/move_to_project_finder.rb | 2 +- app/models/group.rb | 4 +-- app/models/member.rb | 5 ++++ app/models/project.rb | 4 +-- app/models/project_team.rb | 55 ++++++++++++++++++++++++++++------- 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/app/finders/move_to_project_finder.rb b/app/finders/move_to_project_finder.rb index 02592796ddc..9b5b0bebc77 100644 --- a/app/finders/move_to_project_finder.rb +++ b/app/finders/move_to_project_finder.rb @@ -9,7 +9,7 @@ class MoveToProjectFinder projects = @user.authorized_projects projects = projects.search(search) if search.present? projects = skip_projects_before(projects, offset_id.to_i) if offset_id.present? - ProjectTeam.preload_max_member_access(projects.map(&:team)) + ProjectTeam.preload_max_member_access(@user.id, projects.map(&:team)) projects = take_projects(projects) projects.delete(from_project) projects diff --git a/app/models/group.rb b/app/models/group.rb index 37631b99701..843dbe57163 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -6,7 +6,7 @@ class Group < Namespace include AccessRequestable include Referable - has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' + has_many :group_members, -> { merge(Member.non_request) }, dependent: :destroy, as: :source, class_name: 'GroupMember' alias_method :members, :group_members has_many :users, through: :group_members has_many :owners, @@ -14,7 +14,7 @@ class Group < Namespace through: :group_members, source: :user - has_many :requesters, -> { where.not(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' + has_many :requesters, -> { merge(Member.request) }, dependent: :destroy, as: :source, class_name: 'GroupMember' has_many :project_group_links, dependent: :destroy has_many :shared_projects, through: :project_group_links, source: :project diff --git a/app/models/member.rb b/app/models/member.rb index 24ab1276ee9..af70120f83d 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -29,6 +29,7 @@ class Member < ActiveRecord::Base scope :invite, -> { where.not(invite_token: nil) } scope :non_invite, -> { where(invite_token: nil) } + scope :non_request, -> { where(requested_at: nil) } scope :request, -> { where.not(requested_at: nil) } scope :has_access, -> { where('access_level > 0') } @@ -53,6 +54,10 @@ class Member < ActiveRecord::Base default_value_for :notification_level, NotificationSetting.levels[:global] class << self + def access_for_user_id_on_sources(user_id, sources) + where(source: sources, user_id: user_id).non_request.has_access.pluck(:source_id, :access_level).to_h + end + def access_for_user_ids(user_ids) where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h end diff --git a/app/models/project.rb b/app/models/project.rb index a667857d058..032da005e60 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -109,11 +109,11 @@ class Project < ActiveRecord::Base has_many :hooks, dependent: :destroy, class_name: 'ProjectHook' has_many :protected_branches, dependent: :destroy - has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'ProjectMember' + has_many :project_members, -> { merge(Member.non_request) }, dependent: :destroy, as: :source, class_name: 'ProjectMember' alias_method :members, :project_members has_many :users, through: :project_members - has_many :requesters, -> { where.not(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'ProjectMember' + has_many :requesters, -> { merge(Member.request) }, dependent: :destroy, as: :source, class_name: 'ProjectMember' has_many :deploy_keys_projects, dependent: :destroy has_many :deploy_keys, through: :deploy_keys_projects diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 2c630c1f626..660d85a05da 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -6,10 +6,33 @@ class ProjectTeam end class << self - def preload_max_member_access(project_teams) + def preload_max_member_access(user_id, project_teams) projects = project_teams.map(&:project) run_preload(projects, [:namespace, :group]) run_preload(projects.select(&:allowed_to_share_with_group?), [project_group_links: :group]) + load_max_accesses_for_user_id_on(user_id, project_teams) if RequestStore.active? + end + + def load_max_accesses_for_user_id_on(user_id, project_teams) + project_accesses = Member.access_for_user_id_on_sources(user_id, project_teams.map(&:project)) + group_accesses = Member.access_for_user_id_on_sources(user_id, project_teams.map(&:group).compact.uniq) + + project_teams.each do |project_team| + max_access = [ Gitlab::Access::NO_ACCESS, + project_accesses[project_team.project.id], + group_accesses[project_team.project.group.try(:id)] ] + + project = project_team.project + + if project.allowed_to_share_with_group? + project.project_group_links.each do |group_link| + max_access << project_team.max_invited_level_for_users(group_link, user_id) + end + end + + max_access = max_access.compact.max + project_team.cache_max_member_access_for_user_id!(max_access, user_id) + end end def run_preload(projects, associations) @@ -149,13 +172,11 @@ class ProjectTeam # Returns a Hash mapping user ID -> maximum access level. def max_member_access_for_user_ids(user_ids) user_ids = user_ids.uniq - key = "max_member_access:#{project.id}" - access = {} if RequestStore.active? - RequestStore.store[key] ||= {} - access = RequestStore.store[key] + RequestStore.store[max_member_access_key] ||= {} + access = RequestStore.store[max_member_access_key] end # Lookup only the IDs we need @@ -185,12 +206,18 @@ class ProjectTeam access end + def cache_max_member_access_for_user_id!(max_access, user_id) + if RequestStore.active? + RequestStore.store[max_member_access_key] ||= {} + access = RequestStore.store[max_member_access_key] + access[user_id] = max_access + end + end + def max_member_access(user_id) max_member_access_for_user_ids([user_id])[user_id] end - private - # For a given group, return the maximum access level for the user. This is the min of # the invited access level of the group and the access level of the user within the group. # For example, if the group has been given DEVELOPER access but the member has MASTER access, @@ -208,6 +235,16 @@ class ProjectTeam access.each { |key, value| access[key] = [value, capped_access_level].min } end + def group + project.group + end + + private + + def max_member_access_key + @key ||= "max_member_access:#{project.id}" + end + def fetch_members(level = nil) project_members = project.members group_members = group ? group.members : [] @@ -253,10 +290,6 @@ class ProjectTeam User.where(id: user_ids) end - def group - project.group - end - def merge_max!(first_hash, second_hash) first_hash.merge!(second_hash) { |_key, old, new| old > new ? old : new } end -- cgit v1.2.1