Unverified Commit 83232be0 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets
Browse files

Add nested groups support on data level



* add parent_id field to namespaces table to store relation with nested groups
* create routes table to keep information about full path of every group and project
* project/group lookup by full path from routes table
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 8b24814cd582
...@@ -56,7 +56,7 @@ def destroy ...@@ -56,7 +56,7 @@ def destroy
private private
def group def group
@group ||= Group.find_by(path: params[:id]) @group ||= Group.find_by_full_path(params[:id])
end end
def group_params def group_params
......
...@@ -9,7 +9,7 @@ class Groups::ApplicationController < ApplicationController ...@@ -9,7 +9,7 @@ class Groups::ApplicationController < ApplicationController
def group def group
unless @group unless @group
id = params[:group_id] || params[:id] id = params[:group_id] || params[:id]
@group = Group.find_by(path: id) @group = Group.find_by_full_path(id)
unless @group && can?(current_user, :read_group, @group) unless @group && can?(current_user, :read_group, @group)
@group = nil @group = nil
......
...@@ -5,7 +5,7 @@ def can_change_group_visibility_level?(group) ...@@ -5,7 +5,7 @@ def can_change_group_visibility_level?(group)
def group_icon(group) def group_icon(group)
if group.is_a?(String) if group.is_a?(String)
group = Group.find_by(path: group) group = Group.find_by_full_path(group)
end end
group.try(:avatar_url) || image_path('no_group_avatar.png') group.try(:avatar_url) || image_path('no_group_avatar.png')
......
# Store object full path in separate table for easy lookup and uniq validation
# Object must have path db field and respond to full_path and full_path_changed? methods.
module Routable
extend ActiveSupport::Concern
included do
has_one :route, as: :source, autosave: true, dependent: :destroy
validates_associated :route
before_validation :update_route_path, if: :full_path_changed?
end
class_methods do
# Finds a single object by full path match in routes table.
#
# Usage:
#
# Klass.find_by_full_path('gitlab-org/gitlab-ce')
#
# Returns a single object, or nil.
def find_by_full_path(path)
# On MySQL we want to ensure the ORDER BY uses a case-sensitive match so
# any literal matches come first, for this we have to use "BINARY".
# Without this there's still no guarantee in what order MySQL will return
# rows.
binary = Gitlab::Database.mysql? ? 'BINARY' : ''
order_sql = "(CASE WHEN #{binary} routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)"
where_paths_in([path]).reorder(order_sql).take
end
# Builds a relation to find multiple objects by their full paths.
#
# Usage:
#
# Klass.where_paths_in(%w{gitlab-org/gitlab-ce gitlab-org/gitlab-ee})
#
# Returns an ActiveRecord::Relation.
def where_paths_in(paths)
wheres = []
cast_lower = Gitlab::Database.postgresql?
paths.each do |path|
path = connection.quote(path)
where = "(routes.path = #{path})"
if cast_lower
where = "(#{where} OR (LOWER(routes.path) = LOWER(#{path})))"
end
wheres << where
end
if wheres.empty?
none
else
joins(:route).where(wheres.join(' OR '))
end
end
end
private
def update_route_path
route || build_route(source: self)
route.path = full_path
end
end
...@@ -4,12 +4,16 @@ class Namespace < ActiveRecord::Base ...@@ -4,12 +4,16 @@ class Namespace < ActiveRecord::Base
include CacheMarkdownField include CacheMarkdownField
include Sortable include Sortable
include Gitlab::ShellAdapter include Gitlab::ShellAdapter
include Routable
cache_markdown_field :description, pipeline: :description cache_markdown_field :description, pipeline: :description
has_many :projects, dependent: :destroy has_many :projects, dependent: :destroy
belongs_to :owner, class_name: "User" belongs_to :owner, class_name: "User"
belongs_to :parent, class_name: "Namespace"
has_many :children, class_name: "Namespace", foreign_key: :parent_id
validates :owner, presence: true, unless: ->(n) { n.type == "Group" } validates :owner, presence: true, unless: ->(n) { n.type == "Group" }
validates :name, validates :name,
length: { within: 0..255 }, length: { within: 0..255 },
...@@ -86,7 +90,7 @@ def clean_path(path) ...@@ -86,7 +90,7 @@ def clean_path(path)
end end
def to_param def to_param
path full_path
end end
def human_name def human_name
...@@ -150,6 +154,14 @@ def lfs_enabled? ...@@ -150,6 +154,14 @@ def lfs_enabled?
Gitlab.config.lfs.enabled Gitlab.config.lfs.enabled
end end
def full_path
if parent
parent.full_path + '/' + path
else
path
end
end
private private
def repository_storage_paths def repository_storage_paths
...@@ -185,4 +197,8 @@ def refresh_access_of_projects_invited_groups ...@@ -185,4 +197,8 @@ def refresh_access_of_projects_invited_groups
where(projects: { namespace_id: id }). where(projects: { namespace_id: id }).
find_each(&:refresh_members_authorized_projects) find_each(&:refresh_members_authorized_projects)
end end
def full_path_changed?
path_changed? || parent_id_changed?
end
end end
...@@ -14,6 +14,7 @@ class Project < ActiveRecord::Base ...@@ -14,6 +14,7 @@ class Project < ActiveRecord::Base
include TokenAuthenticatable include TokenAuthenticatable
include ProjectFeaturesCompatibility include ProjectFeaturesCompatibility
include SelectForProjectAuthorization include SelectForProjectAuthorization
include Routable
extend Gitlab::ConfigHelper extend Gitlab::ConfigHelper
...@@ -324,87 +325,6 @@ def search_by_title(query) ...@@ -324,87 +325,6 @@ def search_by_title(query)
non_archived.where(table[:name].matches(pattern)) non_archived.where(table[:name].matches(pattern))
end end
# Finds a single project for the given path.
#
# path - The full project path (including namespace path).
#
# Returns a Project, or nil if no project could be found.
def find_with_namespace(path)
namespace_path, project_path = path.split('/', 2)
return unless namespace_path && project_path
namespace_path = connection.quote(namespace_path)
project_path = connection.quote(project_path)
# On MySQL we want to ensure the ORDER BY uses a case-sensitive match so
# any literal matches come first, for this we have to use "BINARY".
# Without this there's still no guarantee in what order MySQL will return
# rows.
binary = Gitlab::Database.mysql? ? 'BINARY' : ''
order_sql = "(CASE WHEN #{binary} namespaces.path = #{namespace_path} " \
"AND #{binary} projects.path = #{project_path} THEN 0 ELSE 1 END)"
where_paths_in([path]).reorder(order_sql).take
end
# Builds a relation to find multiple projects by their full paths.
#
# Each path must be in the following format:
#
# namespace_path/project_path
#
# For example:
#
# gitlab-org/gitlab-ce
#
# Usage:
#
# Project.where_paths_in(%w{gitlab-org/gitlab-ce gitlab-org/gitlab-ee})
#
# This would return the projects with the full paths matching the values
# given.
#
# paths - An Array of full paths (namespace path + project path) for which
# to find the projects.
#
# Returns an ActiveRecord::Relation.
def where_paths_in(paths)
wheres = []
cast_lower = Gitlab::Database.postgresql?
paths.each do |path|
namespace_path, project_path = path.split('/', 2)
next unless namespace_path && project_path
namespace_path = connection.quote(namespace_path)
project_path = connection.quote(project_path)
where = "(namespaces.path = #{namespace_path}
AND projects.path = #{project_path})"
if cast_lower
where = "(
#{where}
OR (
LOWER(namespaces.path) = LOWER(#{namespace_path})
AND LOWER(projects.path) = LOWER(#{project_path})
)
)"
end
wheres << where
end
if wheres.empty?
none
else
joins(:namespace).where(wheres.join(' OR '))
end
end
def visibility_levels def visibility_levels
Gitlab::VisibilityLevel.options Gitlab::VisibilityLevel.options
end end
...@@ -440,6 +360,10 @@ def cached_count ...@@ -440,6 +360,10 @@ def cached_count
def group_ids def group_ids
joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id) joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
end end
# Add alias for Routable method for compatibility with old code.
# In future all calls `find_with_namespace` should be replaced with `find_by_full_path`
alias_method :find_with_namespace, :find_by_full_path
end end
def lfs_enabled? def lfs_enabled?
...@@ -879,13 +803,14 @@ def name_with_namespace ...@@ -879,13 +803,14 @@ def name_with_namespace
end end
alias_method :human_name, :name_with_namespace alias_method :human_name, :name_with_namespace
def path_with_namespace def full_path
if namespace if namespace && path
namespace.path + '/' + path namespace.full_path + '/' + path
else else
path path
end end
end end
alias_method :path_with_namespace, :full_path
def execute_hooks(data, hooks_scope = :push_hooks) def execute_hooks(data, hooks_scope = :push_hooks)
hooks.send(hooks_scope).each do |hook| hooks.send(hooks_scope).each do |hook|
...@@ -1373,4 +1298,8 @@ def default_branch_protected? ...@@ -1373,4 +1298,8 @@ def default_branch_protected?
def validate_board_limit(board) def validate_board_limit(board)
raise BoardLimitExceeded, 'Number of permitted boards exceeded' if boards.size >= NUMBER_OF_PERMITTED_BOARDS raise BoardLimitExceeded, 'Number of permitted boards exceeded' if boards.size >= NUMBER_OF_PERMITTED_BOARDS
end end
def full_path_changed?
path_changed? || namespace_id_changed?
end
end end
class Route < ActiveRecord::Base
belongs_to :source, polymorphic: true
validates :source, presence: true
validates :path,
length: { within: 1..255 },
presence: true,
uniqueness: { case_sensitive: false }
after_update :rename_children, if: :path_changed?
def rename_children
# We update each row separately because MySQL does not have regexp_replace.
# rubocop:disable Rails/FindEach
Route.where('path LIKE ?', "#{path_was}%").each do |route|
# Note that update column skips validation and callbacks.
# We need this to avoid recursive call of rename_children method
route.update_column(:path, route.path.sub(path_was, path))
end
end
end
...@@ -20,6 +20,10 @@ def execute ...@@ -20,6 +20,10 @@ def execute
::Projects::DestroyService.new(project, current_user, skip_repo: true).execute ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
end end
group.children.each do |group|
DestroyGroupService.new(group, current_user).async_execute
end
group.really_destroy! group.really_destroy!
end end
end end
...@@ -13,7 +13,11 @@ ...@@ -13,7 +13,11 @@
- if @forked_project && @forked_project.errors.any? - if @forked_project && @forked_project.errors.any?
%p %p
&ndash; &ndash;
= @forked_project.errors.full_messages.first - error = @forked_project.errors.full_messages.first
- if error.include?("already been taken")
Name has already been taken
- else
= error
%p %p
= link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork", class: "btn" do = link_to new_namespace_project_fork_path(@project.namespace, @project), title: "Fork", class: "btn" do
......
---
title: Add nested groups support on data level
merge_request:
author:
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddParentIdToNamespace < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column(:namespaces, :parent_id, :integer)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddIndexToParentId < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def change
add_concurrent_index(:namespaces, [:parent_id, :id], unique: true)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddRoutesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
create_table :routes do |t|
t.integer :source_id, null: false
t.string :source_type, null: false
t.string :path, null: false
t.timestamps
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 FillRoutesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = true
DOWNTIME_REASON = 'No new namespaces should be created during data copy'
def up
execute <<-EOF
INSERT INTO routes
(source_id, source_type, path)
(SELECT id, 'Namespace', path FROM namespaces)
EOF
end
def down
Route.delete_all(source_type: 'Namespace')
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class FillProjectsRoutesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = true
DOWNTIME_REASON = 'No new projects should be created during data copy'
def up
execute <<-EOF
INSERT INTO routes
(source_id, source_type, path)
(SELECT projects.id, 'Project', concat(namespaces.path, '/', projects.path) FROM projects
INNER JOIN namespaces ON projects.namespace_id = namespaces.id)
EOF
end
def down
Route.delete_all(source_type: 'Project')
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RemoveDuplicatesFromRoutes < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
select_all("SELECT path FROM #{quote_table_name(:routes)} GROUP BY path HAVING COUNT(*) > 1").each do |row|
path = connection.quote(row['path'])
execute(%Q{
DELETE FROM #{quote_table_name(:routes)}
WHERE path = #{path}
AND id != (
SELECT id FROM (
SELECT max(id) AS id
FROM #{quote_table_name(:routes)}
WHERE path = #{path}
) max_ids
)
})
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 AddIndexToRoutes < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def change
add_concurrent_index(:routes, :path, unique: true)
add_concurrent_index(:routes, [:source_type, :source_id], unique: true)
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20161128161412) do ActiveRecord::Schema.define(version: 20161202152035) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -98,14 +98,14 @@ ...@@ -98,14 +98,14 @@
t.text "help_page_text_html" t.text "help_page_text_html"
t.text "shared_runners_text_html" t.text "shared_runners_text_html"
t.text "after_sign_up_text_html" t.text "after_sign_up_text_html"
t.boolean "sidekiq_throttling_enabled", default: false
t.string "sidekiq_throttling_queues"
t.decimal "sidekiq_throttling_factor"
t.boolean "housekeeping_enabled", default: true, null: false t.boolean "housekeeping_enabled", default: true, null: false
t.boolean "housekeeping_bitmaps_enabled", default: true, null: false t.boolean "housekeeping_bitmaps_enabled", default: true, null: false
t.integer "housekeeping_incremental_repack_period", default: 10, null: false t.integer "housekeeping_incremental_repack_period", default: 10, null: false
t.integer "housekeeping_full_repack_period", default: 50, null: false t.integer "housekeeping_full_repack_period", default: 50, null: false
t.integer "housekeeping_gc_period", default: 200, null: false t.integer "housekeeping_gc_period", default: 200, null: false
t.boolean "sidekiq_throttling_enabled", default: false
t.string "sidekiq_throttling_queues"
t.decimal "sidekiq_throttling_factor"
t.boolean "html_emails_enabled", default: true t.boolean "html_emails_enabled", default: true
end end
...@@ -737,8 +737,9 @@ ...@@ -737,8 +737,9 @@
t.integer "visibility_level", default: 20, null: false t.integer "visibility_level", default: 20, null: false
t.boolean "request_access_enabled", default: false, null: false t.boolean "request_access_enabled", default: false, null: false
t.datetime "deleted_at" t.datetime "deleted_at"
t.boolean "lfs_enabled"
t.text "description_html" t.text "description_html"
t.boolean "lfs_enabled"
t.integer "parent_id"
end end
add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree
...@@ -746,6 +747,7 @@ ...@@ -746,6 +747,7 @@
add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree
add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree
add_index "namespaces", ["parent_id", "id"], name: "index_namespaces_on_parent_id_and_id", unique: true, using: :btree
add_index "namespaces", ["path"], name: "index_namespaces_on_path", unique: true, using: :btree add_index "namespaces", ["path"], name: "index_namespaces_on_path", unique: true, using: :btree
add_index "namespaces", ["path"], name: "index_namespaces_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"} add_index "namespaces", ["path"], name: "index_namespaces_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"}
add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree
...@@ -991,6 +993,17 @@ ...@@ -991,6 +993,17 @@