diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2018-02-21 16:42:15 +0300 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2018-02-21 16:42:15 +0300 |
commit | cc3a3fbf892dcdf36c116b8817cf9cb31740fec8 (patch) | |
tree | cb8f8cdbae622ba5b8af33ec6cea36d5f9d72bcb /app | |
parent | 9ff91bc481683e3619e0745b03161a12a5afd497 (diff) | |
parent | 02b9fa0386bb1d96ecd9078da2fb55d8b522f70d (diff) |
Merge branch '42877-snippets-dashboard-slow' into 'master'
Improve query performance for Dashboard::SnippetsController#index
Closes #42877
See merge request gitlab-org/gitlab-ce!17088
Diffstat (limited to 'app')
-rw-r--r-- | app/finders/snippets_finder.rb | 6 | ||||
-rw-r--r-- | app/models/project.rb | 46 |
2 files changed, 39 insertions, 13 deletions
diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index 33359fa1efb..ec61fe1892e 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -56,8 +56,10 @@ class SnippetsFinder < UnionFinder end def feature_available_projects - projects = Project.public_or_visible_to_user(current_user) - .with_feature_available_for_user(:snippets, current_user).select(:id) + projects = Project.public_or_visible_to_user(current_user, use_where_in: false) do |part| + part.with_feature_available_for_user(:snippets, current_user) + end.select(:id) + arel_query = Arel::Nodes::SqlLiteral.new(projects.to_sql) table[:project_id].in(arel_query) end diff --git a/app/models/project.rb b/app/models/project.rb index 2ba6a863500..79058d51af8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -316,18 +316,42 @@ class Project < ActiveRecord::Base # Returns a collection of projects that is either public or visible to the # logged in user. - def self.public_or_visible_to_user(user = nil) - if user - authorized = user - .project_authorizations - .select(1) - .where('project_authorizations.project_id = projects.id') - - levels = Gitlab::VisibilityLevel.levels_for_user(user) - - where('EXISTS (?) OR projects.visibility_level IN (?)', authorized, levels) + # + # A caller may pass in a block to modify individual parts of + # the query, e.g. to apply .with_feature_available_for_user on top of it. + # This is useful for performance as we can stick those additional filters + # at the bottom of e.g. the UNION. + # + # Optionally, turning `use_where_in` off leads to returning a + # relation using #from instead of #where. This can perform much better + # but leads to trouble when used in conjunction with AR's #merge method. + def self.public_or_visible_to_user(user = nil, use_where_in: true, &block) + # If we don't get a block passed, use identity to avoid if/else repetitions + block = ->(part) { part } unless block_given? + + return block.call(public_to_user) unless user + + # If the user is allowed to see all projects, + # we can shortcut and just return. + return block.call(all) if user.full_private_access? + + authorized = user + .project_authorizations + .select(1) + .where('project_authorizations.project_id = projects.id') + authorized_projects = block.call(where('EXISTS (?)', authorized)) + + levels = Gitlab::VisibilityLevel.levels_for_user(user) + visible_projects = block.call(where(visibility_level: levels)) + + # We use a UNION here instead of OR clauses since this results in better + # performance. + union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')]) + + if use_where_in + where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection else - public_to_user + from("(#{union.to_sql}) AS #{table_name}") end end |