diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2017-06-14 14:31:20 +0200 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2017-06-16 13:49:09 +0200 |
commit | 73bf9413b95d20860c09b3b37737c37add2d1342 (patch) | |
tree | 7444d5245c383e639974a22d0282a7255f694f49 /app/models | |
parent | d29347220c07ab0191cf208d3775475c7b5d71ca (diff) | |
download | gitlab-ce-73bf9413b95d20860c09b3b37737c37add2d1342.tar.gz |
Refactor Project.with_feature_available_for_user
This method used to use a UNION, which would lead to it performing the
same query twice; producing less than ideal performance. Further, in
certain cases ActiveRecord could get confused and mess up the variable
bindings, though it's not clear how/why exactly this happens.
Fortunately we can work around all of this by building some of the WHERE
conditions manually, allowing us to use a simple OR statement to get all
the data we want without any of the above problems.
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/project.rb | 26 | ||||
-rw-r--r-- | app/models/project_feature.rb | 7 |
2 files changed, 26 insertions, 7 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 4c394646787..bb183e535d9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -269,17 +269,29 @@ class Project < ActiveRecord::Base # project features may be "disabled", "internal" or "enabled". If "internal", # they are only available to team members. This scope returns projects where # the feature is either enabled, or internal with permission for the user. + # + # This method uses an optimised version of `with_feature_access_level` for + # logged in users to more efficiently get private projects with the given + # feature. def self.with_feature_available_for_user(feature, user) - return with_feature_enabled(feature) if user.try(:admin?) + visible = [nil, ProjectFeature::ENABLED] - unconditional = with_feature_access_level(feature, [nil, ProjectFeature::ENABLED]) - return unconditional if user.nil? + if user&.admin? + with_feature_enabled(feature) + elsif user + column = ProjectFeature.quoted_access_level_column(feature) - conditional = with_feature_access_level(feature, ProjectFeature::PRIVATE) - authorized = user.authorized_projects.merge(conditional.reorder(nil)) + authorized = user.project_authorizations.select(1). + where('project_authorizations.project_id = projects.id') - union = Gitlab::SQL::Union.new([unconditional.select(:id), authorized.select(:id)]) - where(arel_table[:id].in(Arel::Nodes::SqlLiteral.new(union.to_sql))) + with_project_feature. + where("#{column} IN (?) OR (#{column} = ? AND EXISTS (?))", + visible, + ProjectFeature::PRIVATE, + authorized) + else + with_feature_access_level(feature, visible) + end end scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') } diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index e3ef4919b28..dde2a11440d 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -27,6 +27,13 @@ class ProjectFeature < ActiveRecord::Base "#{feature}_access_level".to_sym end + + def quoted_access_level_column(feature) + attribute = connection.quote_column_name(access_level_attribute(feature)) + table = connection.quote_table_name(table_name) + + "#{table}.#{attribute}" + end end # Default scopes force us to unscope here since a service may need to check |