Commit 161d7c02 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Modify namespace name and path validation

Currently namespace name and path have uniq validaiton which does not
allow us to use same group name/path inside different groups. This
commit changes validation in next way:

* Allow same namespace name with different parent_id
* Allow same namespace path. Uniq validation should be handled by routes table
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 3f768f9a
...@@ -17,14 +17,13 @@ class Namespace < ActiveRecord::Base ...@@ -17,14 +17,13 @@ class Namespace < ActiveRecord::Base
validates :owner, presence: true, unless: ->(n) { n.type == "Group" } validates :owner, presence: true, unless: ->(n) { n.type == "Group" }
validates :name, validates :name,
presence: true, presence: true,
uniqueness: true, uniqueness: { scope: :parent_id },
length: { maximum: 255 }, length: { maximum: 255 },
namespace_name: true namespace_name: true
validates :description, length: { maximum: 255 } validates :description, length: { maximum: 255 }
validates :path, validates :path,
presence: true, presence: true,
uniqueness: { case_sensitive: false },
length: { maximum: 255 }, length: { maximum: 255 },
namespace: true namespace: true
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RemoveUniqPathIndexFromNamespace < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
constraint_name = 'namespaces_path_key'
transaction do
if index_exists?(:namespaces, :path)
remove_index(:namespaces, :path)
end
# In some bizarre cases PostgreSQL might have a separate unique constraint
# that we'll need to drop.
if constraint_exists?(constraint_name) && Gitlab::Database.postgresql?
execute("ALTER TABLE namespaces DROP CONSTRAINT IF EXISTS #{constraint_name};")
end
end
end
def down
unless index_exists?(:namespaces, :path)
add_concurrent_index(:namespaces, :path, unique: true)
end
end
def constraint_exists?(name)
indexes(:namespaces).map(&:name).include?(name)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddPathIndexToNamespace < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
add_concurrent_index :namespaces, :path
end
def down
if index_exists?(:namespaces, :path)
remove_index :namespaces, :path
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 RemoveUniqNameIndexFromNamespace < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
constraint_name = 'namespaces_name_key'
transaction do
if index_exists?(:namespaces, :name)
remove_index(:namespaces, :name)
end
# In some bizarre cases PostgreSQL might have a separate unique constraint
# that we'll need to drop.
if constraint_exists?(constraint_name) && Gitlab::Database.postgresql?
execute("ALTER TABLE namespaces DROP CONSTRAINT IF EXISTS #{constraint_name};")
end
end
end
def down
unless index_exists?(:namespaces, :name)
add_concurrent_index(:namespaces, :name, unique: true)
end
end
def constraint_exists?(name)
indexes(:namespaces).map(&:name).include?(name)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddNameIndexToNamespace < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
add_concurrent_index(:namespaces, [:name, :parent_id], unique: true)
end
def down
if index_exists?(:namespaces, :name)
remove_index :namespaces, [:name, :parent_id]
end
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: 20161202152035) do ActiveRecord::Schema.define(version: 20161206153754) 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"
...@@ -744,11 +744,11 @@ ActiveRecord::Schema.define(version: 20161202152035) do ...@@ -744,11 +744,11 @@ ActiveRecord::Schema.define(version: 20161202152035) do
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
add_index "namespaces", ["deleted_at"], name: "index_namespaces_on_deleted_at", using: :btree add_index "namespaces", ["deleted_at"], name: "index_namespaces_on_deleted_at", using: :btree
add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree add_index "namespaces", ["name", "parent_id"], name: "index_namespaces_on_name_and_parent_id", 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", ["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", 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
......
...@@ -50,9 +50,8 @@ describe Group, models: true do ...@@ -50,9 +50,8 @@ describe Group, models: true do
describe 'validations' do describe 'validations' do
it { is_expected.to validate_presence_of :name } it { is_expected.to validate_presence_of :name }
it { is_expected.to validate_uniqueness_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) }
it { is_expected.to validate_presence_of :path } it { is_expected.to validate_presence_of :path }
it { is_expected.to validate_uniqueness_of(:path) }
it { is_expected.not_to validate_presence_of :owner } it { is_expected.not_to validate_presence_of :owner }
end end
......
...@@ -6,20 +6,16 @@ describe Namespace, models: true do ...@@ -6,20 +6,16 @@ describe Namespace, models: true do
it { is_expected.to have_many :projects } it { is_expected.to have_many :projects }
it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_uniqueness_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) }
it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_length_of(:description).is_at_most(255) } it { is_expected.to validate_length_of(:description).is_at_most(255) }
it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_presence_of(:path) }
it { is_expected.to validate_uniqueness_of(:path) }
it { is_expected.to validate_length_of(:path).is_at_most(255) } it { is_expected.to validate_length_of(:path).is_at_most(255) }
it { is_expected.to validate_presence_of(:owner) } it { is_expected.to validate_presence_of(:owner) }
describe "Mass assignment" do
end
describe "Respond to" do describe "Respond to" do
it { is_expected.to respond_to(:human_name) } it { is_expected.to respond_to(:human_name) }
it { is_expected.to respond_to(:to_param) } it { is_expected.to respond_to(:to_param) }
......
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