summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2016-12-14 15:19:17 +0000
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2016-12-14 15:19:17 +0000
commitabad9a9b41f549e9660ac21a978dca4033efa777 (patch)
tree4af12a6ef4f7ead85c2d80a48687f42c492564fa
parentb70b962892b2428557d95a9639fbcead4fe546fd (diff)
parent4bdfb0391a578847fde5589eb44ac6737c9e4b64 (diff)
downloadgitlab-ce-abad9a9b41f549e9660ac21a978dca4033efa777.tar.gz
Merge branch 'dz-remove-namespaces-path-uniq' into 'master'
Modify namespace name and path validation ## What does this MR do? * Allow same namespace name with different parent_id * Allow same namespace path. Uniq validation should be handled by routes table ## Are there points in the code the reviewer needs to double check? migrations ## Why was this MR needed? So we can use same name in different nesting like: ``` gitlab-ce/foo gitlab-com/foo foo/gitlab-ce ``` ## Screenshots (if relevant) no ## Does this MR meet the acceptance criteria? - ~~[Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added~~ - ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - ~~API support added~~ - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if it does - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? https://gitlab.com/gitlab-org/gitlab-ce/issues/2772 See merge request !8013
-rw-r--r--app/models/namespace.rb3
-rw-r--r--db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb36
-rw-r--r--db/migrate/20161206153751_add_path_index_to_namespace.rb20
-rw-r--r--db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb36
-rw-r--r--db/migrate/20161206153754_add_name_index_to_namespace.rb20
-rw-r--r--db/schema.rb6
-rw-r--r--spec/models/group_spec.rb3
-rw-r--r--spec/models/namespace_spec.rb6
8 files changed, 118 insertions, 12 deletions
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 37374044551..f0479d94986 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -17,14 +17,13 @@ class Namespace < ActiveRecord::Base
validates :owner, presence: true, unless: ->(n) { n.type == "Group" }
validates :name,
presence: true,
- uniqueness: true,
+ uniqueness: { scope: :parent_id },
length: { maximum: 255 },
namespace_name: true
validates :description, length: { maximum: 255 }
validates :path,
presence: true,
- uniqueness: { case_sensitive: false },
length: { maximum: 255 },
namespace: true
diff --git a/db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb b/db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb
new file mode 100644
index 00000000000..2977917f2d1
--- /dev/null
+++ b/db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb
@@ -0,0 +1,36 @@
+# 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
diff --git a/db/migrate/20161206153751_add_path_index_to_namespace.rb b/db/migrate/20161206153751_add_path_index_to_namespace.rb
new file mode 100644
index 00000000000..b0bac7d121e
--- /dev/null
+++ b/db/migrate/20161206153751_add_path_index_to_namespace.rb
@@ -0,0 +1,20 @@
+# 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
diff --git a/db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb b/db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb
new file mode 100644
index 00000000000..cc9d4974baa
--- /dev/null
+++ b/db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb
@@ -0,0 +1,36 @@
+# 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
diff --git a/db/migrate/20161206153754_add_name_index_to_namespace.rb b/db/migrate/20161206153754_add_name_index_to_namespace.rb
new file mode 100644
index 00000000000..aaa35ed6f0a
--- /dev/null
+++ b/db/migrate/20161206153754_add_name_index_to_namespace.rb
@@ -0,0 +1,20 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index ae47456084f..4711b7873af 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -744,11 +744,11 @@ ActiveRecord::Schema.define(version: 20161212142807) do
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", ["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", ["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", using: :btree
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
@@ -1290,4 +1290,4 @@ ActiveRecord::Schema.define(version: 20161212142807) do
add_foreign_key "subscriptions", "projects", on_delete: :cascade
add_foreign_key "trending_projects", "projects", on_delete: :cascade
add_foreign_key "u2f_registrations", "users"
-end
+end \ No newline at end of file
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 850b1a3cf1e..893c6827a91 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -50,9 +50,8 @@ describe Group, models: true do
describe 'validations' do
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_uniqueness_of(:path) }
it { is_expected.not_to validate_presence_of :owner }
end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 7f82e85563b..069c59fb5ca 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -6,20 +6,16 @@ describe Namespace, models: true do
it { is_expected.to have_many :projects }
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(:description).is_at_most(255) }
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_presence_of(:owner) }
- describe "Mass assignment" do
- end
-
describe "Respond to" do
it { is_expected.to respond_to(:human_name) }
it { is_expected.to respond_to(:to_param) }