Commit ac382b56 authored by Yorick Peterse's avatar Yorick Peterse

Use CTEs for nested groups and authorizations

This commit introduces the usage of Common Table Expressions (CTEs) to
efficiently retrieve nested group hierarchies, without having to rely on
the "routes" table (which is an _incredibly_ inefficient way of getting
the data). This requires a patch to ActiveRecord (found in the added
initializer) to work properly as ActiveRecord doesn't support WITH
statements properly out of the box.

Unfortunately MySQL provides no efficient way of getting nested groups.
For example, the old routes setup could easily take 5-10 seconds
depending on the amount of "routes" in a database. Providing vastly
different logic for both MySQL and PostgreSQL will negatively impact the
development process. Because of this the various nested groups related
methods return empty relations when used in combination with MySQL.

For project authorizations the logic is split up into two classes:

* Gitlab::ProjectAuthorizations::WithNestedGroups
* Gitlab::ProjectAuthorizations::WithoutNestedGroups

Both classes get the fresh project authorizations (= as they should be
in the "project_authorizations" table), including nested groups if
PostgreSQL is used. The logic of these two classes is quite different
apart from their public interface. This complicates development a bit,
but unfortunately there is no way around this.

This commit also introduces Gitlab::GroupHierarchy. This class can be
used to get the ancestors and descendants of a base relation, or both by
using a UNION. This in turn is used by methods such as:

* Namespace#ancestors
* Namespace#descendants
* User#all_expanded_groups

Again this class relies on CTEs and thus only works on PostgreSQL. The
Namespace methods will return an empty relation when MySQL is used,
while User#all_expanded_groups will return only the groups a user is a
direct member of.

Performance wise the impact is quite large. For example, on GitLab.com
Namespace#descendants used to take around 580 ms to retrieve data for a
particular user. Using CTEs we are able to reduce this down to roughly 1
millisecond, returning the exact same data.

== On The Fly Refreshing

Refreshing of authorizations on the fly (= when
users.authorized_projects_populated was not set) is removed with this
commit. This simplifies the code, and ensures any queries used for
authorizations are not mutated because they are executed in a Rails
scope (e.g. Project.visible_to_user).

This commit includes a migration to schedule refreshing authorizations
for all users, ensuring all of them have their authorizations in place.
Said migration schedules users in batches of 5000, with 5 minutes
between every batch to smear the load around a bit.

== Spec Changes

This commit also introduces some changes to various specs. For example,
some specs for ProjectTeam assumed that creating a personal project
would _not_ lead to the owner having access, which is incorrect. Because
we also no longer refresh authorizations on the fly for new users some
code had to be added to the "empty_project" factory. This chunk of code
ensures that the owner's permissions are refreshed after creating the
project, something that is normally done in Projects::CreateService.
parent e261b4b8
...@@ -84,89 +84,6 @@ module Routable ...@@ -84,89 +84,6 @@ module Routable
joins(:route).where(wheres.join(' OR ')) joins(:route).where(wheres.join(' OR '))
end end
end end
# Builds a relation to find multiple objects that are nested under user membership
#
# Usage:
#
# Klass.member_descendants(1)
#
# Returns an ActiveRecord::Relation.
def member_descendants(user_id)
joins(:route).
joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%')
INNER JOIN members ON members.source_id = r2.source_id
AND members.source_type = r2.source_type").
where('members.user_id = ?', user_id)
end
# Builds a relation to find multiple objects that are nested under user
# membership. Includes the parent, as opposed to `#member_descendants`
# which only includes the descendants.
#
# Usage:
#
# Klass.member_self_and_descendants(1)
#
# Returns an ActiveRecord::Relation.
def member_self_and_descendants(user_id)
joins(:route).
joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%')
OR routes.path = r2.path
INNER JOIN members ON members.source_id = r2.source_id
AND members.source_type = r2.source_type").
where('members.user_id = ?', user_id)
end
# Returns all objects in a hierarchy, where any node in the hierarchy is
# under the user membership.
#
# Usage:
#
# Klass.member_hierarchy(1)
#
# Examples:
#
# Given the following group tree...
#
# _______group_1_______
# | |
# | |
# nested_group_1 nested_group_2
# | |
# | |
# nested_group_1_1 nested_group_2_1
#
#
# ... the following results are returned:
#
# * the user is a member of group 1
# => 'group_1',
# 'nested_group_1', nested_group_1_1',
# 'nested_group_2', 'nested_group_2_1'
#
# * the user is a member of nested_group_2
# => 'group1',
# 'nested_group_2', 'nested_group_2_1'
#
# * the user is a member of nested_group_2_1
# => 'group1',
# 'nested_group_2', 'nested_group_2_1'
#
# Returns an ActiveRecord::Relation.
def member_hierarchy(user_id)
paths = member_self_and_descendants(user_id).pluck('routes.path')
return none if paths.empty?
wheres = paths.map do |path|
"#{connection.quote(path)} = routes.path
OR
#{connection.quote(path)} LIKE CONCAT(routes.path, '/%')"
end
joins(:route).where(wheres.join(' OR '))
end
end end
def full_name def full_name
......
...@@ -3,7 +3,11 @@ module SelectForProjectAuthorization ...@@ -3,7 +3,11 @@ module SelectForProjectAuthorization
module ClassMethods module ClassMethods
def select_for_project_authorization def select_for_project_authorization
select("members.user_id, projects.id AS project_id, members.access_level") select("projects.id AS project_id, members.access_level")
end
def select_as_master_for_project_authorization
select(["projects.id AS project_id", "#{Gitlab::Access::MASTER} AS access_level"])
end end
end end
end end
...@@ -38,6 +38,10 @@ class Group < Namespace ...@@ -38,6 +38,10 @@ class Group < Namespace
after_save :update_two_factor_requirement after_save :update_two_factor_requirement
class << self class << self
def supports_nested_groups?
Gitlab::Database.postgresql?
end
# Searches for groups matching the given query. # Searches for groups matching the given query.
# #
# This method uses ILIKE on PostgreSQL and LIKE on MySQL. # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
...@@ -78,7 +82,7 @@ class Group < Namespace ...@@ -78,7 +82,7 @@ class Group < Namespace
if current_scope.joins_values.include?(:shared_projects) if current_scope.joins_values.include?(:shared_projects)
joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id') joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id')
.where('project_namespace.share_with_group_lock = ?', false) .where('project_namespace.share_with_group_lock = ?', false)
.select("members.user_id, projects.id AS project_id, LEAST(project_group_links.group_access, members.access_level) AS access_level") .select("projects.id AS project_id, LEAST(project_group_links.group_access, members.access_level) AS access_level")
else else
super super
end end
......
...@@ -176,26 +176,22 @@ class Namespace < ActiveRecord::Base ...@@ -176,26 +176,22 @@ class Namespace < ActiveRecord::Base
projects.with_shared_runners.any? projects.with_shared_runners.any?
end end
# Scopes the model on ancestors of the record # Returns all the ancestors of the current namespaces.
def ancestors def ancestors
if parent_id return self.class.none if !Group.supports_nested_groups? || !parent_id
path = route ? route.path : full_path
paths = []
until path.blank? Gitlab::GroupHierarchy.
path = path.rpartition('/').first new(self.class.where(id: parent_id)).
paths << path base_and_ancestors
end end
self.class.joins(:route).where('routes.path IN (?)', paths).reorder('routes.path ASC') # Returns all the descendants of the current namespace.
else
self.class.none
end
end
# Scopes the model on direct and indirect children of the record
def descendants def descendants
self.class.joins(:route).merge(Route.inside_path(route.path)).reorder('routes.path ASC') return self.class.none unless Group.supports_nested_groups?
Gitlab::GroupHierarchy.
new(self.class.where(parent_id: id)).
base_and_descendants
end end
def user_ids_for_project_authorizations def user_ids_for_project_authorizations
......
...@@ -6,6 +6,12 @@ class ProjectAuthorization < ActiveRecord::Base ...@@ -6,6 +6,12 @@ class ProjectAuthorization < ActiveRecord::Base
validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true
validates :user, uniqueness: { scope: [:project, :access_level] }, presence: true validates :user, uniqueness: { scope: [:project, :access_level] }, presence: true
def self.select_from_union(union)
select(['project_id', 'MAX(access_level) AS access_level']).
from("(#{union.to_sql}) #{ProjectAuthorization.table_name}").
group(:project_id)
end
def self.insert_authorizations(rows, per_batch = 1000) def self.insert_authorizations(rows, per_batch = 1000)
rows.each_slice(per_batch) do |slice| rows.each_slice(per_batch) do |slice|
tuples = slice.map do |tuple| tuples = slice.map do |tuple|
......
...@@ -10,9 +10,12 @@ class User < ActiveRecord::Base ...@@ -10,9 +10,12 @@ class User < ActiveRecord::Base
include Sortable include Sortable
include CaseSensitivity include CaseSensitivity
include TokenAuthenticatable include TokenAuthenticatable
include IgnorableColumn
DEFAULT_NOTIFICATION_LEVEL = :participating DEFAULT_NOTIFICATION_LEVEL = :participating
ignore_column :authorized_projects_populated
add_authentication_token_field :authentication_token add_authentication_token_field :authentication_token
add_authentication_token_field :incoming_email_token add_authentication_token_field :incoming_email_token
...@@ -212,7 +215,6 @@ class User < ActiveRecord::Base ...@@ -212,7 +215,6 @@ class User < ActiveRecord::Base
scope :blocked, -> { with_states(:blocked, :ldap_blocked) } scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
scope :external, -> { where(external: true) } scope :external, -> { where(external: true) }
scope :active, -> { with_state(:active).non_internal } scope :active, -> { with_state(:active).non_internal }
scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all }
scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members WHERE user_id IS NOT NULL AND requested_at IS NULL)') } scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members WHERE user_id IS NOT NULL AND requested_at IS NULL)') }
scope :todo_authors, ->(user_id, state) { where(id: Todo.where(user_id: user_id, state: state).select(:author_id)) } scope :todo_authors, ->(user_id, state) { where(id: Todo.where(user_id: user_id, state: state).select(:author_id)) }
scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('last_sign_in_at', 'DESC')) } scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('last_sign_in_at', 'DESC')) }
...@@ -504,23 +506,18 @@ class User < ActiveRecord::Base ...@@ -504,23 +506,18 @@ class User < ActiveRecord::Base
Group.where("namespaces.id IN (#{union.to_sql})") Group.where("namespaces.id IN (#{union.to_sql})")
end end
def nested_groups # Returns a relation of groups the user has access to, including their parent
Group.member_descendants(id) # and child groups (recursively).
end
def all_expanded_groups def all_expanded_groups
Group.member_hierarchy(id) return groups unless Group.supports_nested_groups?
Gitlab::GroupHierarchy.new(groups).all_groups
end end
def expanded_groups_requiring_two_factor_authentication def expanded_groups_requiring_two_factor_authentication
all_expanded_groups.where(require_two_factor_authentication: true) all_expanded_groups.where(require_two_factor_authentication: true)
end end
def nested_groups_projects
Project.joins(:namespace).where('namespaces.parent_id IS NOT NULL').
member_descendants(id)
end
def refresh_authorized_projects def refresh_authorized_projects
Users::RefreshAuthorizedProjectsService.new(self).execute Users::RefreshAuthorizedProjectsService.new(self).execute
end end
...@@ -529,18 +526,15 @@ class User < ActiveRecord::Base ...@@ -529,18 +526,15 @@ class User < ActiveRecord::Base
project_authorizations.where(project_id: project_ids).delete_all project_authorizations.where(project_id: project_ids).delete_all
end end
def set_authorized_projects_column
unless authorized_projects_populated
update_column(:authorized_projects_populated, true)
end
end
def authorized_projects(min_access_level = nil) def authorized_projects(min_access_level = nil)
refresh_authorized_projects unless authorized_projects_populated # We're overriding an association, so explicitly call super with no
# arguments or it would be passed as `force_reload` to the association
# We're overriding an association, so explicitly call super with no arguments or it would be passed as `force_reload` to the association
projects = super() projects = super()
projects = projects.where('project_authorizations.access_level >= ?', min_access_level) if min_access_level
if min_access_level
projects = projects.
where('project_authorizations.access_level >= ?', min_access_level)
end
projects projects
end end
......
...@@ -73,12 +73,11 @@ module Users ...@@ -73,12 +73,11 @@ module Users
# remove - The IDs of the authorization rows to remove. # remove - The IDs of the authorization rows to remove.
# add - Rows to insert in the form `[user id, project id, access level]` # add - Rows to insert in the form `[user id, project id, access level]`
def update_authorizations(remove = [], add = []) def update_authorizations(remove = [], add = [])
return if remove.empty? && add.empty? && user.authorized_projects_populated return if remove.empty? && add.empty?
User.transaction do User.transaction do
user.remove_project_authorizations(remove) unless remove.empty? user.remove_project_authorizations(remove) unless remove.empty?
ProjectAuthorization.insert_authorizations(add) unless add.empty? ProjectAuthorization.insert_authorizations(add) unless add.empty?
user.set_authorized_projects_column
end end
# Since we batch insert authorization rows, Rails' associations may get # Since we batch insert authorization rows, Rails' associations may get
...@@ -101,38 +100,13 @@ module Users ...@@ -101,38 +100,13 @@ module Users
end end
def fresh_authorizations def fresh_authorizations
ProjectAuthorization. klass = if Group.supports_nested_groups?
unscoped. Gitlab::ProjectAuthorizations::WithNestedGroups
select('project_id, MAX(access_level) AS access_level'). else
from("(#{project_authorizations_union.to_sql}) #{ProjectAuthorization.table_name}"). Gitlab::ProjectAuthorizations::WithoutNestedGroups
group(:project_id)
end end
private klass.new(user).calculate
# Returns a union query of projects that the user is authorized to access
def project_authorizations_union
relations = [
# Personal projects
user.personal_projects.select("#{user.id} AS user_id, projects.id AS project_id, #{Gitlab::Access::MASTER} AS access_level"),
# Projects the user is a member of
user.projects.select_for_project_authorization,
# Projects of groups the user is a member of
user.groups_projects.select_for_project_authorization,
# Projects of subgroups of groups the user is a member of
user.nested_groups_projects.select_for_project_authorization,
# Projects shared with groups the user is a member of
user.groups.joins(:shared_projects).select_for_project_authorization,
# Projects shared with subgroups of groups the user is a member of
user.nested_groups.joins(:shared_projects).select_for_project_authorization
]
Gitlab::SQL::Union.new(relations)
end end
end end
end end
# Adds support for WITH statements when using PostgreSQL. The code here is taken
# from https://github.com/shmay/ctes_in_my_pg which at the time of writing has
# not been pushed to RubyGems. The license of this repository is as follows:
#
# The MIT License (MIT)
#
# Copyright (c) 2012 Dan McClain
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
# in the Software without restriction, including without limitation the rights
# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
# copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in
# all copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.
module ActiveRecord
class Relation
class Merger # :nodoc:
def normal_values
NORMAL_VALUES + [:with]
end
end
end
end
module ActiveRecord::Querying
delegate :with, to: :all
end
module ActiveRecord
class Relation
# WithChain objects act as placeholder for queries in which #with does not have any parameter.
# In this case, #with must be chained with #recursive to return a new relation.
class WithChain
def initialize(scope)
@scope = scope
end
# Returns a new relation expressing WITH RECURSIVE
def recursive(*args)
@scope.with_values += args
@scope.recursive_value = true
@scope
end
end
def with_values
@values[:with] || []
end
def with_values=(values)
raise ImmutableRelation if @loaded
@values[:with] = values
end
def recursive_value=(value)
raise ImmutableRelation if @loaded
@values[:recursive] = value
end
def recursive_value
@values[:recursive]
end
def with(opts = :chain, *rest)
if opts == :chain
WithChain.new(spawn)
elsif opts.blank?
self
else
spawn.with!(opts, *rest)
end
end
def with!(opts = :chain, *rest) # :nodoc:
if opts == :chain
WithChain.new(self)
else
self.with_values += [opts] + rest
self
end
end
def build_arel
arel = super()
build_with(arel) if @values[:with]
arel
end
def build_with(arel)
with_statements = with_values.flat_map do |with_value|
case with_value
when String
with_value
when Hash
with_value.map do |name, expression|
case expression
when String
select = Arel::Nodes::SqlLiteral.new "(#{expression})"
when ActiveRecord::Relation, Arel::SelectManager
select = Arel::Nodes::SqlLiteral.new "(#{expression.to_sql})"
end
Arel::Nodes::As.new Arel::Nodes::SqlLiteral.new("\"#{name}\""), select
end
when Arel::Nodes::As
with_value
end
end
unless with_statements.empty?
if recursive_value
arel.with :recursive, with_statements
else
arel.with with_statements
end
end
end
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RescheduleProjectAuthorizations < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
class User < ActiveRecord::Base
self.table_name = 'users'
end
def up
offset = 0
batch = 5000
start = Time.now
loop do
relation = User.where('id > ?', offset)
user_ids = relation.limit(batch).reorder(id: :asc).pluck(:id)
break if user_ids.empty?
offset = user_ids.last
# This will schedule each batch 5 minutes after the previous batch was
# scheduled. This smears out the load over time, instead of immediately
# scheduling a million jobs.
Sidekiq::Client.push_bulk(
'queue' => 'authorized_projects',
'args' => user_ids.zip,
'class' => 'AuthorizedProjectsWorker',
'at' => start.to_i
)
start += 5.minutes
end
end
def down
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RemoveUsersAuthorizedProjectsPopulated < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def change
remove_column :users, :authorized_projects_populated, :boolean
end
end
...@@ -1352,7 +1352,6 @@ ActiveRecord::Schema.define(version: 20170508190732) do ...@@ -1352,7 +1352,6 @@ ActiveRecord::Schema.define(version: 20170508190732) do
t.boolean "external", default: false t.boolean "external", default: false
t.string "incoming_email_token" t.string "incoming_email_token"
t.string "organization" t.string "organization"
t.boolean "authorized_projects_populated"
t.boolean "require_two_factor_authentication_from_group", default: false, null: false t.boolean "require_two_factor_authentication_from_group", default: false, null: false
t.integer "two_factor_grace_period", default: 48, null: false t.integer "two_factor_grace_period", default: 48, null: false
t.boolean "ghost" t.boolean "ghost"
......
module Gitlab
# Retrieving of parent or child groups based on a base ActiveRecord relation.
#
# This class uses recursive CTEs and as a result will only work on PostgreSQL.
class GroupHierarchy
attr_reader :base, :model
# base - An instance of ActiveRecord::Relation for which to get parent or
# child groups.
def initialize(base)
@base = base
@model = base.model
end
# Returns a relation that includes the base set of groups and all their
# ancestors (recursively).
def base_and_ancestors
base_and_ancestors_cte.apply_to(model.all)
end
# Returns a relation that includes the base set of groups and all their
# descendants (recursively).
def base_and_descendants
base_and_descendants_cte.apply_to(model.all)
end
# Returns a relation that includes the base groups, their ancestors, and the
# descendants of the base groups.
#
# The resulting query will roughly look like the following:
#
# WITH RECURSIVE ancestors AS ( ... ),
# descendants AS ( ... )
# SELECT *
# FROM (
# SELECT *
# FROM ancestors namespaces
#
# UNION
#
# SELECT *
# FROM descendants namespaces
# ) groups;
#
# Using this approach allows us to further add criteria to the relation with
# Rails thinking it's selecting data the usual way.
def all_groups
ancestors = base_and_ancestors_cte
descendants = base_and_descendants_cte
ancestors_table = ancestors.alias_to(groups_table)
descendants_table = descendants.alias_to(groups_table)
union = SQL::Union.new([model.unscoped.from(ancestors_table),
model.unscoped.from(descendants_table)])
model.
unscoped.
with.
recursive(ancestors.to_arel, descendants.to_arel).
from("(#{union.to_sql}) #{model.table_name}")
end
private
def base_and_ancestors_cte
cte = SQL::RecursiveCTE.new(:base_and_ancestors)
cte << base.except(:order)
# Recursively get all the ancestors of the base set.
cte << model.
from([groups_table, cte.table]).
where(groups_table[:id].eq(cte.table[:parent_id])).
except(:order)
cte
end
def base_and_descendants_cte
cte = SQL::RecursiveCTE.new(:base_and_descendants)
cte << base.except(:order)
# Recursively get all the descendants of the base set.
cte << model.
from([groups_table, cte.table]).
where(groups_table[:parent_id].eq(cte.table[:id])).
except(:order)
cte
end
def groups_table
model.arel_table
end
end
end
module Gitlab
module ProjectAuthorizations
# Calculating new project authorizations when supporting nested groups.
#
# This class relies on Common Table Expressions to efficiently get all data,
# including data for nested groups. As a result this class can only be used
# on PostgreSQL.
class WithNestedGroups
attr_reader :user
# user - The User object for which to calculate the authorizations.
def initialize(user)
@user = user
end
def calculate
cte = recursive_cte
cte_alias = cte.table.alias(Group.table_name)
projects = Project.arel_table
links = ProjectGroupLink.arel_table
# These queries don't directly use the user object so they don't depend
# on the state of said object, ensuring the produced queries are always
# the same.
relations = [
# The project a user has direct access to.
user.projects.select_for_project_authorization,
# The personal projects of the user.
user.personal_projects.select_as_master_for_project_authorization,
# Projects that belong directly to any of the groups the user has
# access to.
Namespace.
unscoped.
select([alias_as_column(projects[:id], 'project_id'),
cte_alias[:access_level]]).
from(cte_alias).
joins(:projects),
# Projects shared with any of the namespaces the user has access to.
Namespace.
unscoped.
select([links[:project_id],
least(cte_alias[:access_level],
links[:group_access],
'access_level')]).
from(cte_alias).
joins('INNER JOIN project_group_links ON project_group_links.group_id = namespaces.id').
joins('INNER JOIN projects ON projects.id = project_group_links.project_id').
joins('INNER JOIN namespaces p_ns ON p_ns.id = projects.namespace_id').
where('p_ns.share_with_group_lock IS FALSE')
]
union = Gitlab::SQL::Union.new(relations)
ProjectAuthorization.
unscoped.
with.
recursive(cte.to_arel).
select_from_union(union)
end
private
# Builds a recursive CTE that gets all the groups the current user has
# access to, including any nested groups.
def recursive_cte
cte = Gitlab::SQL::RecursiveCTE.new(:namespaces_cte)
members = Member.arel_table
namespaces = Namespace.arel_table
# Namespaces the user is a member of.
cte << user.groups.
select([namespaces[:id], members[:access_level]]).
except(:order)
# Sub groups of any groups the user is a member of.
cte << Group.select([namespaces[:id],
greatest(members[:access_level],
cte.table[:access_level], 'access_level')]).
joins(join_cte(cte)).
joins(join_members).
except(:order)
cte
end
# Builds a LEFT JOIN to join optional memberships onto the CTE.
def join_members
members = Member.arel_table
namespaces = Namespace.arel_table
cond = members[:source_id].
eq(namespaces[:id]).
and(members[:source_type].eq('Namespace')).
and(members[:requested_at].eq(nil)).
and(members[:user_id].eq(user.id))
Arel::Nodes::OuterJoin.new(members, Arel::Nodes::On.new(cond))
end
# Builds an INNER JOIN to join namespaces onto the CTE.
def join_cte(cte)
namespaces = Namespace.arel_table
cond = cte.table[:id].eq(namespaces[:parent_id])
Arel::Nodes::InnerJoin.new(cte.table, Arel::Nodes::On.new(cond))
end
def greatest(left, right, column_alias)
sql_function('GREATEST', [left, right], column_alias)
end
def least(left, right, column_alias)
sql_function('LEAST', [left, right], column_alias)
end
def sql_function(name, args, column_alias)
alias_as_column(Arel::Nodes::NamedFunction.new(name, args), column_alias)
end
def alias_as_column(value, alias_to)
Arel::Nodes::As.new(value, Arel::Nodes::SqlLiteral.new(alias_to))
end
end
end
end
module Gitlab
module ProjectAuthorizations
# Calculating new project authorizations when not supporting nested groups.
class WithoutNestedGroups
attr_reader :user
# user - The User object for which to calculate the authorizations.
def initialize(user)
@user = user
end
def calculate
relations = [
# Projects the user is a direct member of
user.projects.select_for_project_authorization,
# Personal projects
user.personal_projects.select_as_master_for_project_authorization,
# Projects of groups the user is a member of
user.groups_projects.select_for_project_authorization,
# Projects shared with groups the user is a member of
user.groups.joins(:shared_projects).select_for_project_authorization
]
union = Gitlab::SQL::Union.new(relations)
ProjectAuthorization.
unscoped.
select_from_union(union)
end
end
end
end
module Gitlab
module SQL
# Class for easily building recursive CTE statements.
#
# Example:
#
# cte = RecursiveCTE.new(:my_cte_name)
# ns = Arel::Table.new(:namespaces)
#
# cte << Namespace.
# where(ns[:parent_id].eq(some_namespace_id))
#
# cte << Namespace.
# from([ns, cte.table]).
# where(ns[:parent_id].eq(cte.table[:id]))
#
# Namespace.with.
# recursive(cte.to_arel).
# from(cte.alias_to(ns))
class RecursiveCTE
attr_reader :table
# name - The name of the CTE as a String or Symbol.
def initialize(name)
@table = Arel::Table.new(name)
@queries = []
end
# Adds a query to the body of the CTE.
#
# relation - The relation object to add to the body of the CTE.
def <<(relation)
@queries << relation
end
# Returns the Arel relation for this CTE.
def to_arel
sql = Arel::Nodes::SqlLiteral.new(Union.new(@queries).to_sql)
Arel::Nodes::As.new(table, Arel::Nodes::Grouping.new(sql))
end
# Returns an "AS" statement that aliases the CTE name as the given table
# name. This allows one to trick ActiveRecord into thinking it's selecting
# from an actual table, when in reality it's selecting from a CTE.
#
# alias_table - The Arel table to use as the alias.
def alias_to(alias_table)
Arel::Nodes::As.new(table, alias_table)
end
# Applies the CTE to the given relation, returning a new one that will
# query from it.
def apply_to(relation)
relation.except(:where).
with.
recursive(to_arel).
from(alias_to(relation.model.arel_table))
end
end
end
end
...@@ -22,7 +22,7 @@ describe AutocompleteController do ...@@ -22,7 +22,7 @@ describe AutocompleteController do
let(:body) { JSON.parse(response.body) } let(:body) { JSON.parse(response.body) }
it { expect(body).to be_kind_of(Array) } it { expect(body).to be_kind_of(Array) }
it { expect(body.size).to eq 1 } it { expect(body.size).to eq 2 }
it { expect(body.map { |u| u["username"] }).to include(user.username) } it { expect(body.map { |u| u["username"] }).to include(user.username) }
end end
...@@ -80,8 +80,8 @@ describe AutocompleteController do ...@@ -80,8 +80,8 @@ describe AutocompleteController do
end end
it { expect(body).to be_kind_of(Array) } it { expect(body).to be_kind_of(Array) }
it { expect(body.size).to eq 2 } it { expect(body.size).to eq 3 }
it { expect(body.map { |u| u['username'] }).to match_array([user.username, non_member.username]) } it { expect(body.map { |u| u['username'] }).to include(user.username, non_member.username) }
end end
end end
...@@ -108,7 +108,7 @@ describe AutocompleteController do ...@@ -108,7 +108,7 @@ describe AutocompleteController do
end end
it { expect(body).to be_kind_of(Array) } it { expect(body).to be_kind_of(Array) }
it { expect(body.size).to eq 1 } it { expect(body.size).to eq 2 }
end end
describe 'GET #users with project' do describe 'GET #users with project' do
......
...@@ -2,7 +2,7 @@ require 'spec_helper' ...@@ -2,7 +2,7 @@ require 'spec_helper'
describe Projects::MergeRequestsController do describe Projects::MergeRequestsController do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { project.owner }
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
let(:merge_request_with_conflicts) do let(:merge_request_with_conflicts) do
create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) do |mr| create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) do |mr|
...@@ -12,7 +12,6 @@ describe Projects::MergeRequestsController do ...@@ -12,7 +12,6 @@ describe Projects::MergeRequestsController do
before do before do
sign_in(user) sign_in(user)
project.team << [user, :master]
end end
describe 'GET new' do describe 'GET new' do
...@@ -292,6 +291,8 @@ describe Projects::MergeRequestsController do ...@@ -292,6 +291,8 @@ describe Projects::MergeRequestsController do
end end
context 'when user cannot access' do context 'when user cannot access' do
let(:user) { create(:user) }
before do before do
project.add_reporter(user) project.add_reporter(user)
xhr :post, :merge, base_params xhr :post, :merge, base_params
...@@ -448,6 +449,8 @@ describe Projects::MergeRequestsController do ...@@ -448,6 +449,8 @@ describe Projects::MergeRequestsController do
end end
describe "DELETE destroy" do describe "DELETE destroy" do
let(:user) { create(:user) }
it "denies access to users unless they're admin or project owner" do it "denies access to users unless they're admin or project owner" do
delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid
......
...@@ -107,6 +107,18 @@ FactoryGirl.define do ...@@ -107,6 +107,18 @@ FactoryGirl.define do
merge_requests_access_level: merge_requests_access_level, merge_requests_access_level: merge_requests_access_level,
repository_access_level: evaluator.repository_access_level repository_access_level: evaluator.repository_access_level
) )
# Normally the class Projects::CreateService is used for creating
# projects, and this class takes care of making sure the owner and current
# user have access to the project. Our specs don't use said service class,
# thus we must manually refresh things here.
owner = project.owner
if owner && owner.is_a?(User) && !project.pending_delete
project.members.create!(user: owner, access_level: Gitlab::Access::MASTER)
end
project.group&.refresh_members_authorized_projects
end end
end end
......
...@@ -22,7 +22,7 @@ feature 'Group name toggle', feature: true, js: true do ...@@ -22,7 +22,7 @@ feature 'Group name toggle', feature: true, js: true do
expect(page).not_to have_css('.group-name-toggle') expect(page).not_to have_css('.group-name-toggle')
end end
it 'is present if the title is longer than the container' do it 'is present if the title is longer than the container', :nested_groups do
visit group_path(nested_group_3) visit group_path(nested_group_3)
title_width = page.evaluate_script("$('.title')[0].offsetWidth") title_width = page.evaluate_script("$('.title')[0].offsetWidth")
...@@ -35,7 +35,7 @@ feature 'Group name toggle', feature: true, js: true do ...@@ -35,7 +35,7 @@ feature 'Group name toggle', feature: true, js: true do
expect(title_width).to be > container_width expect(title_width).to be > container_width
end end
it 'should show the full group namespace when toggled' do it 'should show the full group namespace when toggled', :nested_groups do
page_height = page.current_window.size[1] page_height = page.current_window.size[1]
page.current_window.resize_to(SMALL_SCREEN, page_height) page.current_window.resize_to(SMALL_SCREEN, page_height)
visit group_path(nested_group_3) visit group_path(nested_group_3)
......
...@@ -12,7 +12,7 @@ feature 'Groups members list', feature: true do ...@@ -12,7 +12,7 @@ feature 'Groups members list', feature: true do
login_as(user1) login_as(user1)
end end
scenario 'show members from current group and parent' do scenario 'show members from current group and parent', :nested_groups do
group.add_developer(user1) group.add_developer(user1)
nested_group.add_developer(user2) nested_group.add_developer(user2)
...@@ -22,7 +22,7 @@ feature 'Groups members list', feature: true do ...@@ -22,7 +22,7 @@ feature 'Groups members list', feature: true do
expect(second_row.text).to include(user2.name) expect(second_row.text).to include(user2.name)
end end
scenario 'show user once if member of both current group and parent' do scenario 'show user once if member of both current group and parent', :nested_groups do
group.add_developer(user1) group.add_developer(user1)
nested_group.add_developer(user1) nested_group.add_developer(user1)
......
...@@ -83,7 +83,7 @@ feature 'Group', feature: true do ...@@ -83,7 +83,7 @@ feature 'Group', feature: true do
end end
end end
describe 'create a nested group', js: true do describe 'create a nested group', :nested_groups, js: true do
let(:group) { create(:group, path: 'foo') } let(:group) { create(:group, path: 'foo') }
context 'as admin' do context 'as admin' do
...@@ -196,7 +196,7 @@ feature 'Group', feature: true do ...@@ -196,7 +196,7 @@ feature 'Group', feature: true do
end end
end end
describe 'group page with nested groups', js: true do describe 'group page with nested groups', :nested_groups, js: true do
let!(:group) { create(:group) } let!(:group) { create(:group) }
let!(:nested_group) { create(:group, parent: group) } let!(:nested_group) { create(:group, parent: group) }
let!(:path) { group_path(group) } let!(:path) { group_path(group) }
......
...@@ -58,7 +58,7 @@ describe 'Dropdown assignee', :feature, :js do ...@@ -58,7 +58,7 @@ describe 'Dropdown assignee', :feature, :js do
it 'should load all the assignees when opened' do it 'should load all the assignees when opened' do
filtered_search.set('assignee:') filtered_search.set('assignee:')
expect(dropdown_assignee_size).to eq(3) expect(dropdown_assignee_size).to eq(4)
end end
it 'shows current user at top of dropdown' do it 'shows current user at top of dropdown' do
......
...@@ -65,7 +65,7 @@ describe 'Dropdown author', js: true, feature: true do ...@@ -65,7 +65,7 @@ describe 'Dropdown author', js: true, feature: true do
it 'should load all the authors when opened' do it 'should load all the authors when opened' do
send_keys_to_filtered_search('author:') send_keys_to_filtered_search('author:')
expect(dropdown_author_size).to eq(3) expect(dropdown_author_size).to eq(4)
end end
it 'shows current user at top of dropdown' do it 'shows current user at top of dropdown' do
......
...@@ -40,7 +40,7 @@ feature 'Project group links', :feature, :js do ...@@ -40,7 +40,7 @@ feature 'Project group links', :feature, :js do
another_group.add_master(master) another_group.add_master(master)
end end
it 'does not show ancestors' do it 'does not show ancestors', :nested_groups do
visit namespace_project_settings_members_path(project.namespace, project) visit namespace_project_settings_members_path(project.namespace, project)
click_link 'Search for a group' click_link 'Search for a group'
......
...@@ -3,10 +3,9 @@ require 'spec_helper' ...@@ -3,10 +3,9 @@ require 'spec_helper'
feature 'Projects > Members > Sorting', feature: true do feature 'Projects > Members > Sorting', feature: true do
let(:master) { create(:user, name: 'John Doe') } let(:master) { create(:user, name: 'John Doe') }
let(:developer) { create(:user, name: 'Mary Jane', last_sign_in_at: 5.days.ago) } let(:developer) { create(:user, name: 'Mary Jane', last_sign_in_at: 5.days.ago) }
let(:project) { create(:empty_project) } let(:project) { create(:empty_project, namespace: master.namespace, creator: master) }
background do background do
create(:project_member, :master, user: master, project: project, created_at: 5.days.ago)
create(:project_member, :developer, user: developer, project: project, created_at: 3.days.ago) create(:project_member, :developer, user: developer, project: project, created_at: 3.days.ago)
login_as(master) login_as(master)
...@@ -39,16 +38,16 @@ feature 'Projects > Members > Sorting', feature: true do ...@@ -39,16 +38,16 @@ feature 'Projects > Members > Sorting', feature: true do
scenario 'sorts by last joined' do scenario 'sorts by last joined' do
visit_members_list(sort: :last_joined) visit_members_list(sort: :last_joined)
expect(first_member).to include(developer.name) expect(first_member).to include(master.name)
expect(second_member).to include(master.name) expect(second_member).to include(developer.name)
expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Last joined') expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Last joined')
end end
scenario 'sorts by oldest joined' do scenario 'sorts by oldest joined' do
visit_members_list(sort: :oldest_joined) visit_members_list(sort: :oldest_joined)
expect(first_member).to include(master.name) expect(first_member).to include(developer.name)
expect(second_member).to include(developer.name) expect(second_member).to include(master.name)
expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Oldest joined') expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Oldest joined')
end end
......
...@@ -2,11 +2,10 @@ require 'spec_helper' ...@@ -2,11 +2,10 @@ require 'spec_helper'
feature 'Projects > Members > User requests access', feature: true do feature 'Projects > Members > User requests access', feature: true do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:master) { create(:user) }
let(:project) { create(:project, :public, :access_requestable) } let(:project) { create(:project, :public, :access_requestable) }
let(:master) { project.owner }
background do background do
project.team << [master, :master]
login_as(user) login_as(user)
visit namespace_project_path(project.namespace, project) visit namespace_project_path(project.namespace, project)
end end
......
...@@ -18,7 +18,7 @@ describe GroupMembersFinder, '#execute' do ...@@ -18,7 +18,7 @@ describe GroupMembersFinder, '#execute' do
expect(result.to_a).to eq([member3, member2, member1]) expect(result.to_a).to eq([member3, member2, member1])
end end
it 'returns members for nested group' do it 'returns members for nested group', :nested_groups do
group.add_master(user2) group.add_master(user2)
nested_group.request_access(user4) nested_group.request_access(user4)
member1 = group.add_master(user1) member1 = group.add_master(user1)
......
...@@ -9,7 +9,7 @@ describe MembersFinder, '#execute' do ...@@ -9,7 +9,7 @@ describe MembersFinder, '#execute' do
let(:user3) { create(:user) } let(:user3) { create(:user) }
let(:user4) { create(:user) } let(:user4) { create(:user) }
it 'returns members for project and parent groups' do it 'returns members for project and parent groups', :nested_groups do
nested_group.request_access(user1) nested_group.request_access(user1)
member1 = group.add_master(user2) member1 = group.add_master(user2)
member2 = nested_group.add_master(user3) member2 = nested_group.add_master(user3)
......
require 'spec_helper' require 'spec_helper'
describe Gitlab::Cache::Ci::ProjectPipelineStatus, :redis do describe Gitlab::Cache::Ci::ProjectPipelineStatus, :redis do
let(:project) { create(:project) } let!(:project) { create(:project) }
let(:pipeline_status) { described_class.new(project) } let(:pipeline_status) { described_class.new(project) }
let(:cache_key) { "projects/#{project.id}/pipeline_status" } let(:cache_key) { "projects/#{project.id}/pipeline_status" }
...@@ -18,7 +18,7 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :redis do ...@@ -18,7 +18,7 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :redis do
let(:sha) { '424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6' } let(:sha) { '424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6' }
let(:ref) { 'master' } let(:ref) { 'master' }
let(:pipeline_info) { { sha: sha, status: status, ref: ref } } let(:pipeline_info) { { sha: sha, status: status, ref: ref } }
let(:project_without_status) { create(:project) } let!(:project_without_status) { create(:project) }
describe '.load_in_batch_for_projects' do describe '.load_in_batch_for_projects' do
it 'preloads pipeline_status on projects' do it 'preloads pipeline_status on projects' do
......
require 'spec_helper'
describe Gitlab::GroupHierarchy, :postgresql do
let!(:parent) { create(:group) }
let!(:child1) { create(:group, parent: parent) }
let!(:child2) { create(:group, parent: child1) }
describe '#base_and_ancestors' do
let(:relation) do
described_class.new(Group.where(id: child2.id)).base_and_ancestors
end
it 'includes the base rows' do
expect(relation).to include(child2)
end
it 'includes all of the ancestors' do
expect(relation).to include(parent, child1)
end
end
describe '#base_and_descendants' do
let(:relation) do
described_class.new(Group.where(id: parent.id)).base_and_descendants
end
it 'includes the base rows' do
expect(relation).to include(parent)
end
it 'includes all the descendants' do
expect(relation).to include(child1, child2)
end
end
describe '#all_groups' do
let(:relation) do
described_class.new(Group.where(id: child1.id)).all_groups
end
it 'includes the base rows' do
expect(relation).to include(child1)
end
it 'includes the ancestors' do
expect(relation).to include(parent)
end
it 'includes the descendants' do
expect(relation).to include(child2)
end
end
end
...@@ -2,9 +2,9 @@ require 'spec_helper' ...@@ -2,9 +2,9 @@ require 'spec_helper'
describe Gitlab::ImportExport::MembersMapper, services: true do describe Gitlab::ImportExport::MembersMapper, services: true do
describe 'map members' do describe 'map members' do
let(:user) { create(:admin, authorized_projects_populated: true) } let(:user) { create(:admin) }
let(:project) { create(:empty_project, :public, name: 'searchable_project') } let(:project) { create(:empty_project, :public, name: 'searchable_project') }
let(:user2) { create(:user, authorized_projects_populated: true) } let(:user2) { create(:user) }
let(:exported_user_id) { 99 } let(:exported_user_id) { 99 }
let(:exported_members) do let(:exported_members) do
[{ [{
...@@ -74,7 +74,7 @@ describe Gitlab::ImportExport::MembersMapper, services: true do ...@@ -74,7 +74,7 @@ describe Gitlab::ImportExport::MembersMapper, services: true do
end end
context 'user is not an admin' do context 'user is not an admin' do
let(:user) { create(:user, authorized_projects_populated: true) } let(:user) { create(:user) }
it 'does not map a project member' do it 'does not map a project member' do
expect(members_mapper.map[exported_user_id]).to eq(user.id) expect(members_mapper.map[exported_user_id]).to eq(user.id)
...@@ -94,7 +94,7 @@ describe Gitlab::ImportExport::MembersMapper, services: true do ...@@ -94,7 +94,7 @@ describe Gitlab::ImportExport::MembersMapper, services: true do
end end
context 'importer same as group member' do context 'importer same as group member' do
let(:user2) { create(:admin, authorized_projects_populated: true) } let(:user2) { create(:admin) }
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:empty_project, :public, name: 'searchable_project', namespace: group) } let(:project) { create(:empty_project, :public, name: 'searchable_project', namespace: group) }
let(:members_mapper) do let(:members_mapper) do
......
require 'spec_helper'
describe Gitlab::ProjectAuthorizations do
let(:group) { create(:group) }
let!(:owned_project) { create(:empty_project) }
let!(:other_project) { create(:empty_project) }
let!(:group_project) { create(:empty_project, namespace: group) }
let(:user) { owned_project.namespace.owner }
def map_access_levels(rows)
rows.each_with_object({}) do |row, hash|
hash[row.project_id] = row.access_level
end
end
before do
other_project.team << [user, :reporter]
group.add_developer(user)
end
let(:authorizations) do
klass = if Group.supports_nested_groups?
Gitlab::ProjectAuthorizations::WithNestedGroups
else
Gitlab::ProjectAuthorizations::WithoutNestedGroups
end
klass.new(user).calculate
end
it 'returns the correct number of authorizations' do
expect(authorizations.length).to eq(3)
end
it 'includes the correct projects' do
expect(authorizations.pluck(:project_id)).
to include(owned_project.id, other_project.id, group_project.id)
end
it 'includes the correct access levels' do
mapping = map_access_levels(authorizations)
expect(mapping[owned_project.id]).to eq(Gitlab::Access::MASTER)
expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER)
expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER)
end
if Group.supports_nested_groups?
context 'with nested groups' do
let!(:nested_group) { create(:group, parent: group) }
let!(:nested_project) { create(:empty_project, namespace: nested_group) }
it 'includes nested groups' do
expect(authorizations.pluck(:project_id)).to include(nested_project.id)
end
it 'inherits access levels when the user is not a member of a nested group' do
mapping = map_access_levels(authorizations)
expect(mapping[nested_project.id]).to eq(Gitlab::Access::DEVELOPER)
end
it 'uses the greatest access level when a user is a member of a nested group' do
nested_group.add_master(user)
mapping = map_access_levels(authorizations)
expect(mapping[nested_project.id]).to eq(Gitlab::Access::MASTER)
end
end
end
end
require 'spec_helper'
describe Gitlab::SQL::RecursiveCTE, :postgresql do
let(:cte) { described_class.new(:cte_name) }
describe '#to_arel' do
it 'generates an Arel relation for the CTE body' do
rel1 = User.where(id: 1)
rel2 = User.where(id: 2)
cte << rel1
cte << rel2
sql = cte.to_arel.to_sql
name = ActiveRecord::Base.connection.quote_table_name(:cte_name)
sql1, sql2 = ActiveRecord::Base.connection.unprepared_statement do
[rel1.except(:order).to_sql, rel2.except(:order).to_sql]
end
expect(sql).to eq("#{name} AS (#{sql1}\nUNION\n#{sql2})")
end
end
describe '#alias_to' do
it 'returns an alias for the CTE' do
table = Arel::Table.new(:kittens)
source_name = ActiveRecord::Base.connection.quote_table_name(:cte_name)
alias_name = ActiveRecord::Base.connection.quote_table_name(:kittens)
expect(cte.alias_to(table).to_sql).to eq("#{source_name} AS #{alias_name}")
end
end
describe '#apply_to' do
it 'applies a CTE to an ActiveRecord::Relation' do
user = create(:user)
cte = described_class.new(:cte_name)
cte << User.where(id: user.id)
relation = cte.apply_to(User.all)
expect(relation.to_sql).to match(/WITH RECURSIVE.+cte_name/)
expect(relation.to_a).to eq(User.where(id: user.id).to_a)
end
end
end
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20170106142508_fill_authorized_projects.rb')
describe FillAuthorizedProjects do
describe '#up' do
it 'schedules the jobs in batches' do
user1 = create(:user)
user2 = create(:user)
expect(Sidekiq::Client).to receive(:push_bulk).with(
'class' => 'AuthorizedProjectsWorker',
'args' => [[user1.id], [user2.id]]
)
described_class.new.up
end
end
end
...@@ -115,123 +115,6 @@ describe Group, 'Routable' do ...@@ -115,123 +115,6 @@ describe Group, 'Routable' do
end end
end end
describe '.member_descendants' do
let!(:user) { create(:user) }
let!(:nested_group) { create(:group, parent: group) }
before { group.add_owner(user) }
subject { described_class.member_descendants(user.id) }
it { is_expected.to eq([nested_group]) }
end
describe '.member_self_and_descendants' do
let!(:user) { create(:user) }
let!(:nested_group) { create(:group, parent: group) }
before { group.add_owner(user) }
subject { described_class.member_self_and_descendants(user.id) }
it { is_expected.to match_array [group, nested_group] }
end
describe '.member_hierarchy' do
# foo/bar would also match foo/barbaz instead of just foo/bar and foo/bar/baz
let!(:user) { create(:user) }
# group
# _______ (foo) _______
# | |
# | |
# nested_group_1 nested_group_2
# (bar) (barbaz)
# | |
# | |
# nested_group_1_1 nested_group_2_1
# (baz) (baz)
#
let!(:nested_group_1) { create :group, parent: group, name: 'bar' }
let!(:nested_group_1_1) { create :group, parent: nested_group_1, name: 'baz' }
let!(:nested_group_2) { create :group, parent: group, name: 'barbaz' }
let!(:nested_group_2_1) { create :group, parent: nested_group_2, name: 'baz' }
context 'user is not a member of any group' do
subject { described_class.member_hierarchy(user.id) }
it 'returns an empty array' do
is_expected.to eq []
end
end
context 'user is member of all groups' do
before do
group.add_owner(user)
nested_group_1.add_owner(user)
nested_group_1_1.add_owner(user)
nested_group_2.add_owner(user)
nested_group_2_1.add_owner(user)
end
subject { described_class.member_hierarchy(user.id) }
it 'returns all groups' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1,
nested_group_2, nested_group_2_1
]
end
end
context 'user is member of the top group' do
before { group.add_owner(user) }
subject { described_class.member_hierarchy(user.id) }
it 'returns all groups' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1,
nested_group_2, nested_group_2_1
]
end
end
context 'user is member of the first child (internal node), branch 1' do
before { nested_group_1.add_owner(user) }
subject { described_class.member_hierarchy(user.id) }
it 'returns the groups in the hierarchy' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1
]
end
end
context 'user is member of the first child (internal node), branch 2' do
before { nested_group_2.add_owner(user) }
subject { described_class.member_hierarchy(user.id) }
it 'returns the groups in the hierarchy' do
is_expected.to match_array [
group,
nested_group_2, nested_group_2_1
]
end
end
context 'user is member of the last child (leaf node)' do
before { nested_group_1_1.add_owner(user) }
subject { described_class.member_hierarchy(user.id) }
it 'returns the groups in the hierarchy' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1
]
end
end
end
describe '#full_path' do describe '#full_path' do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:nested_group) { create(:group, parent: group) } let(:nested_group) { create(:group, parent: group) }
......
...@@ -340,7 +340,7 @@ describe Group, models: true do ...@@ -340,7 +340,7 @@ describe Group, models: true do
it { expect(subject.parent).to be_kind_of(Group) } it { expect(subject.parent).to be_kind_of(Group) }
end end
describe '#members_with_parents' do describe '#members_with_parents', :nested_groups do
let!(:group) { create(:group, :nested) } let!(:group) { create(:group, :nested) }
let!(:master) { group.parent.add_user(create(:user), GroupMember::MASTER) } let!(:master) { group.parent.add_user(create(:user), GroupMember::MASTER) }
let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) }
......
...@@ -22,16 +22,15 @@ describe ProjectMember, models: true do ...@@ -22,16 +22,15 @@ describe ProjectMember, models: true do
end end
describe '.add_user' do describe '.add_user' do
context 'when called with the project owner' do
it 'adds the user as a member' do it 'adds the user as a member' do
user = create(:user)
project = create(:empty_project) project = create(:empty_project)
expect(project.users).not_to include(project.owner) expect(project.users).not_to include(user)
described_class.add_user(project, project.owner, :master, current_user: project.owner) described_class.add_user(project, user, :master, current_user: project.owner)
expect(project.users.reload).to include(project.owner) expect(project.users.reload).to include(user)
end
end end
end end
......
...@@ -287,21 +287,21 @@ describe Namespace, models: true do ...@@ -287,21 +287,21 @@ describe Namespace, models: true do
end end
end end
describe '#ancestors' do describe '#ancestors', :nested_groups do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:nested_group) { create(:group, parent: group) } let(:nested_group) { create(:group, parent: group) }
let(:deep_nested_group) { create(:group, parent: nested_group) } let(:deep_nested_group) { create(:group, parent: nested_group) }
let(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } let(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
it 'returns the correct ancestors' do it 'returns the correct ancestors' do
expect(very_deep_nested_group.ancestors).to eq([group, nested_group, deep_nested_group]) expect(very_deep_nested_group.ancestors).to include(group, nested_group, deep_nested_group)
expect(deep_nested_group.ancestors).to eq([group, nested_group]) expect(deep_nested_group.ancestors).to include(group, nested_group)
expect(nested_group.ancestors).to eq([group]) expect(nested_group.ancestors).to include(group)
expect(group.ancestors).to eq([]) expect(group.ancestors).to eq([])
end end
end end
describe '#descendants' do describe '#descendants', :nested_groups do
let!(:group) { create(:group, path: 'git_lab') } let!(:group) { create(:group, path: 'git_lab') }
let!(:nested_group) { create(:group, parent: group) } let!(:nested_group) { create(:group, parent: group) }
let!(:deep_nested_group) { create(:group, parent: nested_group) } let!(:deep_nested_group) { create(:group, parent: nested_group) }
...@@ -311,9 +311,9 @@ describe Namespace, models: true do ...@@ -311,9 +311,9 @@ describe Namespace, models: true do
it 'returns the correct descendants' do it 'returns the correct descendants' do
expect(very_deep_nested_group.descendants.to_a).to eq([]) expect(very_deep_nested_group.descendants.to_a).to eq([])
expect(deep_nested_group.descendants.to_a).to eq([very_deep_nested_group]) expect(deep_nested_group.descendants.to_a).to include(very_deep_nested_group)
expect(nested_group.descendants.to_a).to eq([deep_nested_group, very_deep_nested_group]) expect(nested_group.descendants.to_a).to include(deep_nested_group, very_deep_nested_group)
expect(group.descendants.to_a).to eq([nested_group, deep_nested_group, very_deep_nested_group]) expect(group.descendants.to_a).to include(nested_group, deep_nested_group, very_deep_nested_group)
end end
end end
......
...@@ -23,7 +23,7 @@ describe ProjectGroupLink do ...@@ -23,7 +23,7 @@ describe ProjectGroupLink do
expect(project_group_link).not_to be_valid expect(project_group_link).not_to be_valid
end end
it "doesn't allow a project to be shared with an ancestor of the group it is in" do it "doesn't allow a project to be shared with an ancestor of the group it is in", :nested_groups do
project_group_link.group = parent_group project_group_link.group = parent_group
expect(project_group_link).not_to be_valid expect(project_group_link).not_to be_valid
......
...@@ -81,7 +81,7 @@ describe ProjectTeam, models: true do ...@@ -81,7 +81,7 @@ describe ProjectTeam, models: true do
user = create(:user) user = create(:user)
project.add_guest(user) project.add_guest(user)
expect(project.team.members).to contain_exactly(user) expect(project.team.members).to contain_exactly(user, project.owner)
end end
it 'returns project members of a specified level' do it 'returns project members of a specified level' do
...@@ -100,7 +100,8 @@ describe ProjectTeam, models: true do ...@@ -100,7 +100,8 @@ describe ProjectTeam, models: true do
group_access: Gitlab::Access::GUEST group_access: Gitlab::Access::GUEST
) )
expect(project.team.members).to contain_exactly(group_member.user) expect(project.team.members).
to contain_exactly(group_member.user, project.owner)
end end
it 'returns invited members of a group of a specified level' do it 'returns invited members of a group of a specified level' do
...@@ -137,7 +138,10 @@ describe ProjectTeam, models: true do ...@@ -137,7 +138,10 @@ describe ProjectTeam, models: true do
describe '#find_member' do describe '#find_member' do
context 'personal project' do context 'personal project' do
let(:project) { create(:empty_project, :public, :access_requestable) } let(:project) do
create(:empty_project, :public, :access_requestable)
end
let(:requester) { create(:user) } let(:requester) { create(:user) }
before do before do
...@@ -200,7 +204,9 @@ describe ProjectTeam, models: true do ...@@ -200,7 +204,9 @@ describe ProjectTeam, models: true do
let(:requester) { create(:user) } let(:requester) { create(:user) }
context 'personal project' do context 'personal project' do
let(:project) { create(:empty_project, :public, :access_requestable) } let(:project) do
create(:empty_project, :public, :access_requestable)
end
context 'when project is not shared with group' do context 'when project is not shared with group' do
before do before do
...@@ -244,7 +250,9 @@ describe ProjectTeam, models: true do ...@@ -244,7 +250,9 @@ describe ProjectTeam, models: true do
context 'group project' do context 'group project' do
let(:group) { create(:group, :access_requestable) } let(:group) { create(:group, :access_requestable) }
let!(:project) { create(:empty_project, group: group) } let!(:project) do
create(:empty_project, group: group)
end
before do before do
group.add_master(master) group.add_master(master)
...@@ -265,8 +273,15 @@ describe ProjectTeam, models: true do ...@@ -265,8 +273,15 @@ describe ProjectTeam, models: true do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:developer) { create(:user) } let(:developer) { create(:user) }
let(:master) { create(:user) } let(:master) { create(:user) }
let(:personal_project) { create(:empty_project, namespace: developer.namespace) }
let(:group_project) { create(:empty_project, namespace: group) } let(:personal_project) do
create(:empty_project, namespace: developer.namespace)
end
let(:group_project) do
create(:empty_project, namespace: group)
end
let(:members_project) { create(:empty_project) } let(:members_project) { create(:empty_project) }
let(:shared_project) { create(:empty_project) } let(:shared_project) { create(:empty_project) }
......
...@@ -611,16 +611,6 @@ describe User, models: true do ...@@ -611,16 +611,6 @@ describe User, models: true do
it { expect(User.without_projects).to include user_without_project2 } it { expect(User.without_projects).to include user_without_project2 }
end end
describe '.not_in_project' do
before do
User.delete_all
@user = create :user
@project = create(:empty_project)
end
it { expect(User.not_in_project(@project)).to include(@user, @project.owner) }
end
describe 'user creation' do describe 'user creation' do
describe 'normal user' do describe 'normal user' do
let(:user) { create(:user, name: 'John Smith') } let(:user) { create(:user, name: 'John Smith') }
...@@ -1535,48 +1525,103 @@ describe User, models: true do ...@@ -1535,48 +1525,103 @@ describe User, models: true do
end end
end end
describe '#nested_groups' do describe '#all_expanded_groups' do
# foo/bar would also match foo/barbaz instead of just foo/bar and foo/bar/baz
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:group) { create(:group) }
let!(:nested_group) { create(:group, parent: group) }
# group
# _______ (foo) _______
# | |
# | |
# nested_group_1 nested_group_2
# (bar) (barbaz)
# | |
# | |
# nested_group_1_1 nested_group_2_1
# (baz) (baz)
#
let!(:group) { create :group }
let!(:nested_group_1) { create :group, parent: group, name: 'bar' }
let!(:nested_group_1_1) { create :group, parent: nested_group_1, name: 'baz' }
let!(:nested_group_2) { create :group, parent: group, name: 'barbaz' }
let!(:nested_group_2_1) { create :group, parent: nested_group_2, name: 'baz' }
subject { user.all_expanded_groups }
context 'user is not a member of any group' do
it 'returns an empty array' do
is_expected.to eq([])
end
end
context 'user is member of all groups' do
before do before do
group.add_owner(user) group.add_owner(user)
nested_group_1.add_owner(user)
# Add more data to ensure method does not include wrong groups nested_group_1_1.add_owner(user)
create(:group).add_owner(create(:user)) nested_group_2.add_owner(user)
nested_group_2_1.add_owner(user)
end end
it { expect(user.nested_groups).to eq([nested_group]) } it 'returns all groups' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1,
nested_group_2, nested_group_2_1
]
end
end end
describe '#all_expanded_groups' do context 'user is member of the top group' do
let!(:user) { create(:user) } before { group.add_owner(user) }
let!(:group) { create(:group) }
let!(:nested_group_1) { create(:group, parent: group) }
let!(:nested_group_2) { create(:group, parent: group) }
if Group.supports_nested_groups?
it 'returns all groups' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1,
nested_group_2, nested_group_2_1
]
end
else
it 'returns the top-level groups' do
is_expected.to match_array [group]
end
end
end
context 'user is member of the first child (internal node), branch 1', :nested_groups do
before { nested_group_1.add_owner(user) } before { nested_group_1.add_owner(user) }
it { expect(user.all_expanded_groups).to match_array [group, nested_group_1] } it 'returns the groups in the hierarchy' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1
]
end
end end
describe '#nested_groups_projects' do context 'user is member of the first child (internal node), branch 2', :nested_groups do
let!(:user) { create(:user) } before { nested_group_2.add_owner(user) }
let!(:group) { create(:group) }
let!(:nested_group) { create(:group, parent: group) }
let!(:project) { create(:empty_project, namespace: group) }
let!(:nested_project) { create(:empty_project, namespace: nested_group) }
before do
group.add_owner(user)
# Add more data to ensure method does not include wrong projects it 'returns the groups in the hierarchy' do
other_project = create(:empty_project, namespace: create(:group, :nested)) is_expected.to match_array [
other_project.add_developer(create(:user)) group,
nested_group_2, nested_group_2_1
]
end end
end
context 'user is member of the last child (leaf node)', :nested_groups do
before { nested_group_1_1.add_owner(user) }
it { expect(user.nested_groups_projects).to eq([nested_project]) } it 'returns the groups in the hierarchy' do
is_expected.to match_array [
group,
nested_group_1, nested_group_1_1
]
end
end
end end
describe '#refresh_authorized_projects', redis: true do describe '#refresh_authorized_projects', redis: true do
...@@ -1596,10 +1641,6 @@ describe User, models: true do ...@@ -1596,10 +1641,6 @@ describe User, models: true do
expect(user.project_authorizations.count).to eq(2) expect(user.project_authorizations.count).to eq(2)
end end
it 'sets the authorized_projects_populated column' do
expect(user.authorized_projects_populated).to eq(true)
end
it 'stores the correct access levels' do it 'stores the correct access levels' do
expect(user.project_authorizations.where(access_level: Gitlab::Access::GUEST).exists?).to eq(true) expect(user.project_authorizations.where(access_level: Gitlab::Access::GUEST).exists?).to eq(true)
expect(user.project_authorizations.where(access_level: Gitlab::Access::REPORTER).exists?).to eq(true) expect(user.project_authorizations.where(access_level: Gitlab::Access::REPORTER).exists?).to eq(true)
...@@ -1709,7 +1750,7 @@ describe User, models: true do ...@@ -1709,7 +1750,7 @@ describe User, models: true do
end end
end end
context 'with 2FA requirement on nested parent group' do context 'with 2FA requirement on nested parent group', :nested_groups do
let!(:group1) { create :group, require_two_factor_authentication: true } let!(:group1) { create :group, require_two_factor_authentication: true }
let!(:group1a) { create :group, require_two_factor_authentication: false, parent: group1 } let!(:group1a) { create :group, require_two_factor_authentication: false, parent: group1 }
...@@ -1724,7 +1765,7 @@ describe User, models: true do ...@@ -1724,7 +1765,7 @@ describe User, models: true do
end end
end end
context 'with 2FA requirement on nested child group' do context 'with 2FA requirement on nested child group', :nested_groups do
let!(:group1) { create :group, require_two_factor_authentication: false } let!(:group1) { create :group, require_two_factor_authentication: false }
let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 } let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 }
......
...@@ -107,7 +107,7 @@ describe GroupPolicy, models: true do ...@@ -107,7 +107,7 @@ describe GroupPolicy, models: true do
end end
end end
describe 'private nested group inherit permissions' do describe 'private nested group inherit permissions', :nested_groups do
let(:nested_group) { create(:group, :private, parent: group) } let(:nested_group) { create(:group, :private, parent: group) }
subject { described_class.abilities(current_user, nested_group).to_set } subject { described_class.abilities(current_user, nested_group).to_set }
......
...@@ -5,7 +5,6 @@ describe API::Commits do ...@@ -5,7 +5,6 @@ describe API::Commits do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:user2) { create(:user) } let(:user2) { create(:user) }
let!(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } let!(:project) { create(:project, :repository, creator: user, namespace: user.namespace) }
let!(:master) { create(:project_member, :master, user: user, project: project) }
let!(:guest) { create(:project_member, :guest, user: user2, project: project) } let!(:guest) { create(:project_member, :guest, user: user2, project: project) }
let!(:note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') } let!(:note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') }
let!(:another_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'another comment on a commit') } let!(:another_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'another comment on a commit') }
......
...@@ -429,7 +429,7 @@ describe API::Groups do ...@@ -429,7 +429,7 @@ describe API::Groups do
expect(json_response["request_access_enabled"]).to eq(group[:request_access_enabled]) expect(json_response["request_access_enabled"]).to eq(group[:request_access_enabled])
end end
it "creates a nested group" do it "creates a nested group", :nested_groups do
parent = create(:group) parent = create(:group)
parent.add_owner(user3) parent.add_owner(user3)
group = attributes_for(:group, { parent_id: parent.id }) group = attributes_for(:group, { parent_id: parent.id })
......
...@@ -11,8 +11,7 @@ describe API::Projects do ...@@ -11,8 +11,7 @@ describe API::Projects do
let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) } let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) }
let(:project2) { create(:empty_project, path: 'project2', creator_id: user.id, namespace: user.namespace) } let(:project2) { create(:empty_project, path: 'project2', creator_id: user.id, namespace: user.namespace) }
let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') } let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') }
let(:project_member) { create(:project_member, :master, user: user, project: project) } let(:project_member) { create(:project_member, :developer, user: user3, project: project) }
let(:project_member2) { create(:project_member, :developer, user: user3, project: project) }
let(:user4) { create(:user) } let(:user4) { create(:user) }
let(:project3) do let(:project3) do
create(:project, create(:project,
...@@ -27,7 +26,7 @@ describe API::Projects do ...@@ -27,7 +26,7 @@ describe API::Projects do
builds_enabled: false, builds_enabled: false,
snippets_enabled: false) snippets_enabled: false)
end end
let(:project_member3) do let(:project_member2) do
create(:project_member, create(:project_member,
user: user4, user: user4,
project: project3, project: project3,
...@@ -210,7 +209,7 @@ describe API::Projects do ...@@ -210,7 +209,7 @@ describe API::Projects do
let(:public_project) { create(:empty_project, :public) } let(:public_project) { create(:empty_project, :public) }
before do before do
project_member2 project_member
user3.update_attributes(starred_projects: [project, project2, project3, public_project]) user3.update_attributes(starred_projects: [project, project2, project3, public_project])
end end
...@@ -784,19 +783,18 @@ describe API::Projects do ...@@ -784,19 +783,18 @@ describe API::Projects do
describe 'GET /projects/:id/users' do describe 'GET /projects/:id/users' do
shared_examples_for 'project users response' do shared_examples_for 'project users response' do
it 'returns the project users' do it 'returns the project users' do
member = create(:user)
create(:project_member, :developer, user: member, project: project)
get api("/projects/#{project.id}/users", current_user) get api("/projects/#{project.id}/users", current_user)
user = project.namespace.owner
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(response).to include_pagination_headers expect(response).to include_pagination_headers
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response.size).to eq(1) expect(json_response.size).to eq(1)
first_user = json_response.first first_user = json_response.first
expect(first_user['username']).to eq(member.username) expect(first_user['username']).to eq(user.username)
expect(first_user['name']).to eq(member.name) expect(first_user['name']).to eq(user.name)
expect(first_user.keys).to contain_exactly(*%w[name username id state avatar_url web_url]) expect(first_user.keys).to contain_exactly(*%w[name username id state avatar_url web_url])
end end
end end
...@@ -1091,8 +1089,8 @@ describe API::Projects do ...@@ -1091,8 +1089,8 @@ describe API::Projects do
before { user4 } before { user4 }
before { project3 } before { project3 }
before { project4 } before { project4 }
before { project_member3 }
before { project_member2 } before { project_member2 }
before { project_member }
it 'returns 400 when nothing sent' do it 'returns 400 when nothing sent' do
project_param = {} project_param = {}
...@@ -1573,7 +1571,7 @@ describe API::Projects do ...@@ -1573,7 +1571,7 @@ describe API::Projects do
context 'when authenticated as developer' do context 'when authenticated as developer' do
before do before do
project_member2 project_member
end end
it 'returns forbidden error' do it 'returns forbidden error' do
......
...@@ -5,7 +5,6 @@ describe API::V3::Commits do ...@@ -5,7 +5,6 @@ describe API::V3::Commits do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:user2) { create(:user) } let(:user2) { create(:user) }
let!(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } let!(:project) { create(:project, :repository, creator: user, namespace: user.namespace) }
let!(:master) { create(:project_member, :master, user: user, project: project) }
let!(:guest) { create(:project_member, :guest, user: user2, project: project) } let!(:guest) { create(:project_member, :guest, user: user2, project: project) }
let!(:note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') } let!(:note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') }
let!(:another_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'another comment on a commit') } let!(:another_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'another comment on a commit') }
......
...@@ -421,7 +421,7 @@ describe API::V3::Groups do ...@@ -421,7 +421,7 @@ describe API::V3::Groups do
expect(json_response["request_access_enabled"]).to eq(group[:request_access_enabled]) expect(json_response["request_access_enabled"]).to eq(group[:request_access_enabled])
end end
it "creates a nested group" do it "creates a nested group", :nested_groups do
parent = create(:group) parent = create(:group)
parent.add_owner(user3) parent.add_owner(user3)
group = attributes_for(:group, { parent_id: parent.id }) group = attributes_for(:group, { parent_id: parent.id })
......
...@@ -10,8 +10,7 @@ describe API::V3::Projects do ...@@ -10,8 +10,7 @@ describe API::V3::Projects do
let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) } let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) }
let(:project2) { create(:empty_project, path: 'project2', creator_id: user.id, namespace: user.namespace) } let(:project2) { create(:empty_project, path: 'project2', creator_id: user.id, namespace: user.namespace) }
let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') } let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') }
let(:project_member) { create(:project_member, :master, user: user, project: project) } let(:project_member) { create(:project_member, :developer, user: user3, project: project) }
let(:project_member2) { create(:project_member, :developer, user: user3, project: project) }
let(:user4) { create(:user) } let(:user4) { create(:user) }
let(:project3) do let(:project3) do
create(:project, create(:project,
...@@ -25,7 +24,7 @@ describe API::V3::Projects do ...@@ -25,7 +24,7 @@ describe API::V3::Projects do
issues_enabled: false, wiki_enabled: false, issues_enabled: false, wiki_enabled: false,
snippets_enabled: false) snippets_enabled: false)
end end
let(:project_member3) do let(:project_member2) do
create(:project_member, create(:project_member,
user: user4, user: user4,
project: project3, project: project3,
...@@ -286,7 +285,7 @@ describe API::V3::Projects do ...@@ -286,7 +285,7 @@ describe API::V3::Projects do
let(:public_project) { create(:empty_project, :public) } let(:public_project) { create(:empty_project, :public) }
before do before do
project_member2 project_member
user3.update_attributes(starred_projects: [project, project2, project3, public_project]) user3.update_attributes(starred_projects: [project, project2, project3, public_project])
end end
...@@ -622,7 +621,6 @@ describe API::V3::Projects do ...@@ -622,7 +621,6 @@ describe API::V3::Projects do
context 'when authenticated' do context 'when authenticated' do
before do before do
project project
project_member
end end
it 'returns a project by id' do it 'returns a project by id' do
...@@ -814,8 +812,7 @@ describe API::V3::Projects do ...@@ -814,8 +812,7 @@ describe API::V3::Projects do
describe 'GET /projects/:id/users' do describe 'GET /projects/:id/users' do
shared_examples_for 'project users response' do shared_examples_for 'project users response' do
it 'returns the project users' do it 'returns the project users' do
member = create(:user) member = project.owner
create(:project_member, :developer, user: member, project: project)
get v3_api("/projects/#{project.id}/users", current_user) get v3_api("/projects/#{project.id}/users", current_user)
...@@ -1163,8 +1160,8 @@ describe API::V3::Projects do ...@@ -1163,8 +1160,8 @@ describe API::V3::Projects do
before { user4 } before { user4 }
before { project3 } before { project3 }
before { project4 } before { project4 }
before { project_member3 }
before { project_member2 } before { project_member2 }
before { project_member }
context 'when unauthenticated' do context 'when unauthenticated' do
it 'returns authentication error' do it 'returns authentication error' do
......
...@@ -70,7 +70,7 @@ describe Projects::DestroyService, services: true do ...@@ -70,7 +70,7 @@ describe Projects::DestroyService, services: true do
end end
end end
expect(project.team.members.count).to eq 1 expect(project.team.members.count).to eq 2
end end
end end
......
require 'spec_helper' require 'spec_helper'
describe Users::RefreshAuthorizedProjectsService do describe Users::RefreshAuthorizedProjectsService do
let(:project) { create(:empty_project) } # We're using let! here so that any expectations for the service class are not
# triggered twice.
let!(:project) { create(:empty_project) }
let(:user) { project.namespace.owner } let(:user) { project.namespace.owner }
let(:service) { described_class.new(user) } let(:service) { described_class.new(user) }
def create_authorization(project, user, access_level = Gitlab::Access::MASTER)
ProjectAuthorization.
create!(project: project, user: user, access_level: access_level)
end
describe '#execute', :redis do describe '#execute', :redis do
it 'refreshes the authorizations using a lease' do it 'refreshes the authorizations using a lease' do
expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain). expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).
...@@ -31,7 +29,8 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -31,7 +29,8 @@ describe Users::RefreshAuthorizedProjectsService do
it 'updates the authorized projects of the user' do it 'updates the authorized projects of the user' do
project2 = create(:empty_project) project2 = create(:empty_project)
to_remove = create_authorization(project2, user) to_remove = user.project_authorizations.
create!(project: project2, access_level: Gitlab::Access::MASTER)
expect(service).to receive(:update_authorizations). expect(service).to receive(:update_authorizations).
with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]]) with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]])
...@@ -40,7 +39,10 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -40,7 +39,10 @@ describe Users::RefreshAuthorizedProjectsService do
end end
it 'sets the access level of a project to the highest available level' do it 'sets the access level of a project to the highest available level' do
to_remove = create_authorization(project, user, Gitlab::Access::DEVELOPER) user.project_authorizations.delete_all
to_remove = user.project_authorizations.
create!(project: project, access_level: Gitlab::Access::DEVELOPER)
expect(service).to receive(:update_authorizations). expect(service).to receive(:update_authorizations).
with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]]) with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]])
...@@ -61,34 +63,10 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -61,34 +63,10 @@ describe Users::RefreshAuthorizedProjectsService do
service.update_authorizations([], []) service.update_authorizations([], [])
end end
context 'when the authorized projects column is not set' do
before do
user.update!(authorized_projects_populated: nil)
end
it 'populates the authorized projects column' do
service.update_authorizations([], [])
expect(user.authorized_projects_populated).to eq true
end
end
context 'when the authorized projects column is set' do
before do
user.update!(authorized_projects_populated: true)
end
it 'does nothing' do
expect(user).not_to receive(:set_authorized_projects_column)
service.update_authorizations([], [])
end
end
end end
it 'removes authorizations that should be removed' do it 'removes authorizations that should be removed' do
authorization = create_authorization(project, user) authorization = user.project_authorizations.find_by(project_id: project.id)
service.update_authorizations([authorization.project_id]) service.update_authorizations([authorization.project_id])
...@@ -96,6 +74,8 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -96,6 +74,8 @@ describe Users::RefreshAuthorizedProjectsService do
end end
it 'inserts authorizations that should be added' do it 'inserts authorizations that should be added' do
user.project_authorizations.delete_all
service.update_authorizations([], [[user.id, project.id, Gitlab::Access::MASTER]]) service.update_authorizations([], [[user.id, project.id, Gitlab::Access::MASTER]])
authorizations = user.project_authorizations authorizations = user.project_authorizations
...@@ -105,16 +85,6 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -105,16 +85,6 @@ describe Users::RefreshAuthorizedProjectsService do
expect(authorizations[0].project_id).to eq(project.id) expect(authorizations[0].project_id).to eq(project.id)
expect(authorizations[0].access_level).to eq(Gitlab::Access::MASTER) expect(authorizations[0].access_level).to eq(Gitlab::Access::MASTER)
end end
it 'populates the authorized projects column' do
# make sure we start with a nil value no matter what the default in the
# factory may be.
user.update!(authorized_projects_populated: nil)
service.update_authorizations([], [[user.id, project.id, Gitlab::Access::MASTER]])
expect(user.authorized_projects_populated).to eq(true)
end
end end
describe '#fresh_access_levels_per_project' do describe '#fresh_access_levels_per_project' do
...@@ -163,7 +133,7 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -163,7 +133,7 @@ describe Users::RefreshAuthorizedProjectsService do
end end
end end
context 'projects of subgroups of groups the user is a member of' do context 'projects of subgroups of groups the user is a member of', :nested_groups do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:nested_group) { create(:group, parent: group) } let(:nested_group) { create(:group, parent: group) }
let!(:other_project) { create(:empty_project, group: nested_group) } let!(:other_project) { create(:empty_project, group: nested_group) }
...@@ -191,7 +161,7 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -191,7 +161,7 @@ describe Users::RefreshAuthorizedProjectsService do
end end
end end
context 'projects shared with subgroups of groups the user is a member of' do context 'projects shared with subgroups of groups the user is a member of', :nested_groups do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:nested_group) { create(:group, parent: group) } let(:nested_group) { create(:group, parent: group) }
let(:other_project) { create(:empty_project) } let(:other_project) { create(:empty_project) }
...@@ -208,8 +178,6 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -208,8 +178,6 @@ describe Users::RefreshAuthorizedProjectsService do
end end
describe '#current_authorizations_per_project' do describe '#current_authorizations_per_project' do
before { create_authorization(project, user) }
let(:hash) { service.current_authorizations_per_project } let(:hash) { service.current_authorizations_per_project }
it 'returns a Hash' do it 'returns a Hash' do
...@@ -233,13 +201,13 @@ describe Users::RefreshAuthorizedProjectsService do ...@@ -233,13 +201,13 @@ describe Users::RefreshAuthorizedProjectsService do
describe '#current_authorizations' do describe '#current_authorizations' do
context 'without authorizations' do context 'without authorizations' do
it 'returns an empty list' do it 'returns an empty list' do
user.project_authorizations.delete_all
expect(service.current_authorizations.empty?).to eq(true) expect(service.current_authorizations.empty?).to eq(true)
end end
end end
context 'with an authorization' do context 'with an authorization' do
before { create_authorization(project, user) }
let(:row) { service.current_authorizations.take } let(:row) { service.current_authorizations.take }
it 'returns the currently authorized projects' do it 'returns the currently authorized projects' do
......
...@@ -93,6 +93,14 @@ RSpec.configure do |config| ...@@ -93,6 +93,14 @@ RSpec.configure do |config|
Gitlab::Redis.with(&:flushall) Gitlab::Redis.with(&:flushall)
Sidekiq.redis(&:flushall) Sidekiq.redis(&:flushall)
end end
config.around(:each, :nested_groups) do |example|
example.run if Gitlab::GroupHierarchy.supports_nested_groups?
end
config.around(:each, :postgresql) do |example|
example.run if Gitlab::Database.postgresql?
end
end end
FactoryGirl::SyntaxRunner.class_eval do FactoryGirl::SyntaxRunner.class_eval do
......
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