Commit c9e277ee authored by Yorick Peterse's avatar Yorick Peterse

Refactor GroupProjectsFinder#init_collection

This optimises how GroupProjectsFinder builds it collection, producing
simpler and faster queries in the process. It also cleans up the code a
bit to make it easier to understand.
parent 73bf9413
......@@ -33,7 +33,8 @@ class EventsFinder
private
def by_current_user_access(events)
events.merge(ProjectsFinder.new(current_user: current_user).execute).references(:project)
events.merge(ProjectsFinder.new(current_user: current_user).execute).
joins(:project)
end
def by_action(events)
......
......@@ -29,35 +29,69 @@ class GroupProjectsFinder < ProjectsFinder
private
def init_collection
only_owned = options.fetch(:only_owned, false)
only_shared = options.fetch(:only_shared, false)
projects = if current_user
collection_with_user
else
collection_without_user
end
projects = []
union(projects)
end
if current_user
if group.users.include?(current_user)
projects << group.projects unless only_shared
projects << group.shared_projects unless only_owned
def collection_with_user
if group.users.include?(current_user)
if only_shared?
[shared_projects]
elsif only_owned?
[owned_projects]
else
unless only_shared
projects << group.projects.visible_to_user(current_user)
projects << group.projects.public_to_user(current_user)
end
unless only_owned
projects << group.shared_projects.visible_to_user(current_user)
projects << group.shared_projects.public_to_user(current_user)
end
[shared_projects, owned_projects]
end
else
projects << group.projects.public_only unless only_shared
projects << group.shared_projects.public_only unless only_owned
if only_shared?
[shared_projects.public_or_visible_to_user(current_user)]
elsif only_owned?
[owned_projects.public_or_visible_to_user(current_user)]
else
[
owned_projects.public_or_visible_to_user(current_user),
shared_projects.public_or_visible_to_user(current_user)
]
end
end
end
projects
def collection_without_user
if only_shared?
[shared_projects.public_only]
elsif only_owned?
[owned_projects.public_only]
else
[shared_projects.public_only, owned_projects.public_only]
end
end
def union(items)
find_union(items, Project)
if items.one?
items.first
else
find_union(items, Project)
end
end
def only_owned?
options.fetch(:only_owned, false)
end
def only_shared?
options.fetch(:only_shared, false)
end
def owned_projects
group.projects
end
def shared_projects
group.shared_projects
end
end
......@@ -58,27 +58,11 @@ class ProjectsFinder < UnionFinder
if private_only?
current_user.authorized_projects
else
collection_with_user_and_public_projects
Project.public_or_visible_to_user(current_user)
end
end
end
# Builds a collection for a signed in user that includes additional projects
# such as public and internal ones.
#
# This method manually constructs some WHERE conditions in order to ensure the
# produced query is as efficient as possible.
def collection_with_user_and_public_projects
levels = Gitlab::VisibilityLevel.levels_for_user(current_user)
authorized = current_user.project_authorizations.
select(1).
where('project_id = projects.id')
Project.where('EXISTS (?) OR projects.visibility_level IN (?)',
authorized,
levels)
end
# Builds a collection for an anonymous user.
def collection_without_user
if private_only? || owned_projects?
......
......@@ -266,6 +266,23 @@ class Project < ActiveRecord::Base
enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
# 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)
else
public_to_user
end
end
# 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.
......
......@@ -2060,4 +2060,36 @@ describe Project, models: true do
expect(project.last_repository_updated_at.to_i).to eq(project.created_at.to_i)
end
end
describe '.public_or_visible_to_user' do
let!(:user) { create(:user) }
let!(:private_project) do
create(:empty_project, :private, creator: user, namespace: user.namespace)
end
let!(:public_project) { create(:empty_project, :public) }
context 'with a user' do
let(:projects) do
Project.all.public_or_visible_to_user(user)
end
it 'includes projects the user has access to' do
expect(projects).to include(private_project)
end
it 'includes projects the user can see' do
expect(projects).to include(public_project)
end
end
context 'without a user' do
it 'only includes public projects' do
projects = Project.all.public_or_visible_to_user
expect(projects).to eq([public_project])
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment