Commit 37583c1b authored by Jan Provaznik's avatar Jan Provaznik

Merge branch '340495_create_project_namespace_on_project_create' into 'master'

Create ProjectNamespace when a Project is created

See merge request gitlab-org/gitlab!70972
parents 83deb44d c51c36ed
...@@ -497,6 +497,10 @@ class Namespace < ApplicationRecord ...@@ -497,6 +497,10 @@ class Namespace < ApplicationRecord
Feature.enabled?(:block_issue_repositioning, self, type: :ops, default_enabled: :yaml) Feature.enabled?(:block_issue_repositioning, self, type: :ops, default_enabled: :yaml)
end end
def project_namespace_creation_enabled?
Feature.enabled?(:create_project_namespace_on_project_create, self, default_enabled: :yaml)
end
private private
def expire_child_caches def expire_child_caches
......
...@@ -4,8 +4,6 @@ module Namespaces ...@@ -4,8 +4,6 @@ module Namespaces
class ProjectNamespace < Namespace class ProjectNamespace < Namespace
has_one :project, foreign_key: :project_namespace_id, inverse_of: :project_namespace has_one :project, foreign_key: :project_namespace_id, inverse_of: :project_namespace
validates :project, presence: true
def self.sti_name def self.sti_name
'Project' 'Project'
end end
......
...@@ -98,7 +98,7 @@ class Project < ApplicationRecord ...@@ -98,7 +98,7 @@ class Project < ApplicationRecord
before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? } before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? }
before_save :ensure_runners_token before_save :ensure_runners_token
before_save :ensure_project_namespace_in_sync before_validation :ensure_project_namespace_in_sync
after_save :update_project_statistics, if: :saved_change_to_namespace_id? after_save :update_project_statistics, if: :saved_change_to_namespace_id?
...@@ -147,7 +147,7 @@ class Project < ApplicationRecord ...@@ -147,7 +147,7 @@ class Project < ApplicationRecord
belongs_to :namespace belongs_to :namespace
# Sync deletion via DB Trigger to ensure we do not have # Sync deletion via DB Trigger to ensure we do not have
# a project without a project_namespace (or vice-versa) # a project without a project_namespace (or vice-versa)
belongs_to :project_namespace, autosave: true, class_name: 'Namespaces::ProjectNamespace', foreign_key: 'project_namespace_id', inverse_of: :project belongs_to :project_namespace, autosave: true, class_name: 'Namespaces::ProjectNamespace', foreign_key: 'project_namespace_id'
alias_method :parent, :namespace alias_method :parent, :namespace
alias_attribute :parent_id, :namespace_id alias_attribute :parent_id, :namespace_id
...@@ -476,6 +476,7 @@ class Project < ApplicationRecord ...@@ -476,6 +476,7 @@ class Project < ApplicationRecord
validates :project_feature, presence: true validates :project_feature, presence: true
validates :namespace, presence: true validates :namespace, presence: true
validates :project_namespace, presence: true, if: -> { self.namespace && self.root_namespace.project_namespace_creation_enabled? }
validates :name, uniqueness: { scope: :namespace_id } validates :name, uniqueness: { scope: :namespace_id }
validates :import_url, public_url: { schemes: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS }, validates :import_url, public_url: { schemes: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS }, ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
...@@ -2919,13 +2920,29 @@ class Project < ApplicationRecord ...@@ -2919,13 +2920,29 @@ class Project < ApplicationRecord
end end
def ensure_project_namespace_in_sync def ensure_project_namespace_in_sync
if changes.keys & [:name, :path, :namespace_id, :visibility_level] && project_namespace.present? # create project_namespace when project is created if create_project_namespace_on_project_create FF is enabled
build_project_namespace if project_namespace_creation_enabled?
# regardless of create_project_namespace_on_project_create FF we need
# to keep project and project namespace in sync if there is one
sync_attributes(project_namespace) if sync_project_namespace?
end
def project_namespace_creation_enabled?
new_record? && !project_namespace && self.namespace && self.root_namespace.project_namespace_creation_enabled?
end
def sync_project_namespace?
(changes.keys & %w(name path namespace_id namespace visibility_level shared_runners_enabled)).any? && project_namespace.present?
end
def sync_attributes(project_namespace)
project_namespace.name = name project_namespace.name = name
project_namespace.path = path project_namespace.path = path
project_namespace.parent = namespace project_namespace.parent = namespace
project_namespace.shared_runners_enabled = shared_runners_enabled
project_namespace.visibility_level = visibility_level project_namespace.visibility_level = visibility_level
end end
end
end end
Project.prepend_mod_with('Project') Project.prepend_mod_with('Project')
---
name: create_project_namespace_on_project_create
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70972
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344954
milestone: '14.5'
type: development
group: group::workspace
default_enabled: false
...@@ -157,16 +157,17 @@ class Gitlab::Seeder::CycleAnalytics ...@@ -157,16 +157,17 @@ class Gitlab::Seeder::CycleAnalytics
end end
def create_new_vsm_project def create_new_vsm_project
namespace = FactoryBot.create(
:group,
name: "Value Stream Management Group #{suffix}",
path: "vsmg-#{suffix}"
)
project = FactoryBot.create( project = FactoryBot.create(
:project, :project,
name: "Value Stream Management Project #{suffix}", name: "Value Stream Management Project #{suffix}",
path: "vsmp-#{suffix}", path: "vsmp-#{suffix}",
creator: admin, creator: admin,
namespace: FactoryBot.create( namespace: namespace
:group,
name: "Value Stream Management Group #{suffix}",
path: "vsmg-#{suffix}"
)
) )
project.create_repository project.create_repository
......
...@@ -13,7 +13,7 @@ RSpec.describe API::Namespaces do ...@@ -13,7 +13,7 @@ RSpec.describe API::Namespaces do
let_it_be(:ultimate_plan) { create(:ultimate_plan) } let_it_be(:ultimate_plan) { create(:ultimate_plan) }
let_it_be(:project) { create(:project, namespace: group2) } let_it_be(:project) { create(:project, namespace: group2) }
let_it_be(:project) { create(:project, namespace: group2, name: group2.name, path: group2.path) } let_it_be(:project) { create(:project, namespace: group2, name: group2.name, path: group2.path) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project_namespace) { project.project_namespace }
describe "GET /namespaces" do describe "GET /namespaces" do
context "when authenticated as admin" do context "when authenticated as admin" do
......
...@@ -98,7 +98,7 @@ RSpec.describe Projects::DestroyService do ...@@ -98,7 +98,7 @@ RSpec.describe Projects::DestroyService do
end end
context 'when project has an associated ProjectNamespace' do context 'when project has an associated ProjectNamespace' do
let!(:project_namespace) { create(:project_namespace, project: project) } let!(:project_namespace) { project.project_namespace }
it 'destroys the associated ProjectNamespace also' do it 'destroys the associated ProjectNamespace also' do
subject.execute subject.execute
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
FactoryBot.define do FactoryBot.define do
factory :project_namespace, class: 'Namespaces::ProjectNamespace' do factory :project_namespace, class: 'Namespaces::ProjectNamespace' do
project association :project, factory: :project, strategy: :build
parent { project.namespace } parent { project.namespace }
visibility_level { project.visibility_level } visibility_level { project.visibility_level }
name { project.name } name { project.name }
......
...@@ -38,7 +38,8 @@ RSpec.describe Gitlab::Database::Count::ReltuplesCountStrategy do ...@@ -38,7 +38,8 @@ RSpec.describe Gitlab::Database::Count::ReltuplesCountStrategy do
it 'returns nil counts for inherited tables' do it 'returns nil counts for inherited tables' do
models.each { |model| expect(model).not_to receive(:count) } models.each { |model| expect(model).not_to receive(:count) }
expect(subject).to eq({ Namespace => 3 }) # 3 Namespaces as parents for each Project and 3 ProjectNamespaces(for each Project)
expect(subject).to eq({ Namespace => 6 })
end end
end end
......
...@@ -47,7 +47,8 @@ RSpec.describe Gitlab::Database::Count::TablesampleCountStrategy do ...@@ -47,7 +47,8 @@ RSpec.describe Gitlab::Database::Count::TablesampleCountStrategy do
result = subject result = subject
expect(result[Project]).to eq(3) expect(result[Project]).to eq(3)
expect(result[Group]).to eq(1) expect(result[Group]).to eq(1)
expect(result[Namespace]).to eq(4) # 1-Group, 3 namespaces for each project and 3 project namespaces for each project
expect(result[Namespace]).to eq(7)
end end
end end
......
...@@ -425,6 +425,9 @@ RSpec.describe Gitlab::PathRegex do ...@@ -425,6 +425,9 @@ RSpec.describe Gitlab::PathRegex do
it { is_expected.not_to match('gitlab.org/') } it { is_expected.not_to match('gitlab.org/') }
it { is_expected.not_to match('/gitlab.org') } it { is_expected.not_to match('/gitlab.org') }
it { is_expected.not_to match('gitlab git') } it { is_expected.not_to match('gitlab git') }
it { is_expected.not_to match('gitlab?') }
it { is_expected.to match('gitlab.org-') }
it { is_expected.to match('gitlab.org_') }
end end
describe '.project_path_format_regex' do describe '.project_path_format_regex' do
...@@ -437,6 +440,14 @@ RSpec.describe Gitlab::PathRegex do ...@@ -437,6 +440,14 @@ RSpec.describe Gitlab::PathRegex do
it { is_expected.not_to match('?gitlab') } it { is_expected.not_to match('?gitlab') }
it { is_expected.not_to match('git lab') } it { is_expected.not_to match('git lab') }
it { is_expected.not_to match('gitlab.git') } it { is_expected.not_to match('gitlab.git') }
it { is_expected.not_to match('gitlab?') }
it { is_expected.not_to match('gitlab git') }
it { is_expected.to match('gitlab.org') }
it { is_expected.to match('gitlab.org-') }
it { is_expected.to match('gitlab.org_') }
it { is_expected.to match('gitlab.org.') }
it { is_expected.not_to match('gitlab.org/') }
it { is_expected.not_to match('/gitlab.org') }
end end
context 'repository routes' do context 'repository routes' do
......
...@@ -25,7 +25,7 @@ RSpec.describe LoadedInGroupList do ...@@ -25,7 +25,7 @@ RSpec.describe LoadedInGroupList do
context 'with project namespaces' do context 'with project namespaces' do
let_it_be(:group1) { create(:group, parent: parent) } let_it_be(:group1) { create(:group, parent: parent) }
let_it_be(:group2) { create(:group, parent: parent) } let_it_be(:group2) { create(:group, parent: parent) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project_namespace) { project.project_namespace }
it 'does not include project_namespaces in the count of subgroups' do it 'does not include project_namespaces in the count of subgroups' do
expect(found_group.preloaded_subgroup_count).to eq(3) expect(found_group.preloaded_subgroup_count).to eq(3)
......
...@@ -566,7 +566,7 @@ RSpec.describe Group do ...@@ -566,7 +566,7 @@ RSpec.describe Group do
context 'when project namespace exists in the group' do context 'when project namespace exists in the group' do
let!(:project) { create(:project, group: group) } let!(:project) { create(:project, group: group) }
let!(:project_namespace) { create(:project_namespace, project: project) } let!(:project_namespace) { project.project_namespace }
it 'filters out project namespace' do it 'filters out project namespace' do
expect(group.descendants.find_by_id(project_namespace.id)).to be_nil expect(group.descendants.find_by_id(project_namespace.id)).to be_nil
......
...@@ -32,9 +32,10 @@ RSpec.describe Namespace do ...@@ -32,9 +32,10 @@ RSpec.describe Namespace do
describe '#children' do describe '#children' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) } let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:project_namespace) { create(:project_namespace, parent: group) } let_it_be(:project_with_namespace) { create(:project, namespace: group) }
it 'excludes project namespaces' do it 'excludes project namespaces' do
expect(project_with_namespace.project_namespace.parent).to eq(group)
expect(group.children).to match_array([subgroup]) expect(group.children).to match_array([subgroup])
end end
end end
...@@ -239,7 +240,9 @@ RSpec.describe Namespace do ...@@ -239,7 +240,9 @@ RSpec.describe Namespace do
let(:namespace) { build(:project_namespace) } let(:namespace) { build(:project_namespace) }
it 'allows to update path to single char' do it 'allows to update path to single char' do
namespace = create(:project_namespace) project = create(:project)
namespace = project.project_namespace
namespace.update!(path: 'j') namespace.update!(path: 'j')
expect(namespace).to be_valid expect(namespace).to be_valid
...@@ -342,9 +345,13 @@ RSpec.describe Namespace do ...@@ -342,9 +345,13 @@ RSpec.describe Namespace do
describe '.without_project_namespaces' do describe '.without_project_namespaces' do
let_it_be(:user_namespace) { create(:user_namespace) } let_it_be(:user_namespace) { create(:user_namespace) }
let_it_be(:project_namespace) { create(:project_namespace) } let_it_be(:project) { create(:project) }
let_it_be(:project_namespace) { project.project_namespace }
it 'excludes project namespaces' do it 'excludes project namespaces' do
expect(project_namespace).not_to be_nil
expect(project_namespace.parent).not_to be_nil
expect(described_class.all).to include(project_namespace)
expect(described_class.without_project_namespaces).to match_array([namespace, namespace1, namespace2, namespace1sub, namespace2sub, user_namespace, project_namespace.parent]) expect(described_class.without_project_namespaces).to match_array([namespace, namespace1, namespace2, namespace1sub, namespace2sub, user_namespace, project_namespace.parent])
end end
end end
...@@ -562,7 +569,7 @@ RSpec.describe Namespace do ...@@ -562,7 +569,7 @@ RSpec.describe Namespace do
context 'with project namespaces' do context 'with project namespaces' do
let_it_be(:project) { create(:project, namespace: parent_group, path: 'some-new-path') } let_it_be(:project) { create(:project, namespace: parent_group, path: 'some-new-path') }
let_it_be(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project_namespace) { project.project_namespace }
it 'does not return project namespace' do it 'does not return project namespace' do
search_result = described_class.search('path') search_result = described_class.search('path')
......
...@@ -15,7 +15,7 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do ...@@ -15,7 +15,7 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do
# using delete rather than destroy due to `delete` skipping AR hooks/callbacks # using delete rather than destroy due to `delete` skipping AR hooks/callbacks
# so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key # so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project_namespace) { project.project_namespace }
it 'also deletes the associated project' do it 'also deletes the associated project' do
project_namespace.delete project_namespace.delete
......
...@@ -16,7 +16,7 @@ RSpec.describe Project, factory_default: :keep do ...@@ -16,7 +16,7 @@ RSpec.describe Project, factory_default: :keep do
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:group) }
it { is_expected.to belong_to(:namespace) } it { is_expected.to belong_to(:namespace) }
it { is_expected.to belong_to(:project_namespace).class_name('Namespaces::ProjectNamespace').with_foreign_key('project_namespace_id').inverse_of(:project) } it { is_expected.to belong_to(:project_namespace).class_name('Namespaces::ProjectNamespace').with_foreign_key('project_namespace_id') }
it { is_expected.to belong_to(:creator).class_name('User') } it { is_expected.to belong_to(:creator).class_name('User') }
it { is_expected.to belong_to(:pool_repository) } it { is_expected.to belong_to(:pool_repository) }
it { is_expected.to have_many(:users) } it { is_expected.to have_many(:users) }
...@@ -191,7 +191,7 @@ RSpec.describe Project, factory_default: :keep do ...@@ -191,7 +191,7 @@ RSpec.describe Project, factory_default: :keep do
# using delete rather than destroy due to `delete` skipping AR hooks/callbacks # using delete rather than destroy due to `delete` skipping AR hooks/callbacks
# so it's ensured to work at the DB level. Uses AFTER DELETE trigger. # so it's ensured to work at the DB level. Uses AFTER DELETE trigger.
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project_namespace) { project.project_namespace }
it 'also deletes the associated ProjectNamespace' do it 'also deletes the associated ProjectNamespace' do
project.delete project.delete
...@@ -233,6 +233,58 @@ RSpec.describe Project, factory_default: :keep do ...@@ -233,6 +233,58 @@ RSpec.describe Project, factory_default: :keep do
expect(project.project_setting).to be_an_instance_of(ProjectSetting) expect(project.project_setting).to be_an_instance_of(ProjectSetting)
expect(project.project_setting).to be_new_record expect(project.project_setting).to be_new_record
end end
context 'with project namespaces' do
it 'automatically creates a project namespace' do
project = build(:project, path: 'hopefully-valid-path1')
project.save!
expect(project).to be_persisted
expect(project.project_namespace).to be_persisted
expect(project.project_namespace).to be_in_sync_with_project(project)
end
context 'with FF disabled' do
before do
stub_feature_flags(create_project_namespace_on_project_create: false)
end
it 'does not create a project namespace' do
project = build(:project, path: 'hopefully-valid-path2')
project.save!
expect(project).to be_persisted
expect(project.project_namespace).to be_nil
end
end
end
end
context 'updating a project' do
context 'with project namespaces' do
it 'keeps project namespace in sync with project' do
project = create(:project)
project.update!(path: 'hopefully-valid-path1')
expect(project).to be_persisted
expect(project.project_namespace).to be_persisted
expect(project.project_namespace).to be_in_sync_with_project(project)
end
context 'with FF disabled' do
before do
stub_feature_flags(create_project_namespace_on_project_create: false)
end
it 'does not create a project namespace when project is updated' do
project = create(:project)
project.update!(path: 'hopefully-valid-path1')
expect(project).to be_persisted
expect(project.project_namespace).to be_nil
end
end
end
end end
context 'updating cd_cd_settings' do context 'updating cd_cd_settings' do
...@@ -322,6 +374,18 @@ RSpec.describe Project, factory_default: :keep do ...@@ -322,6 +374,18 @@ RSpec.describe Project, factory_default: :keep do
create(:project) create(:project)
end end
context 'validates project namespace creation' do
it 'does not create project namespace if project is not created' do
project = build(:project, path: 'tree')
project.valid?
expect(project).not_to be_valid
expect(project).to be_new_record
expect(project.project_namespace).to be_new_record
end
end
context 'repository storages inclusion' do context 'repository storages inclusion' do
let(:project2) { build(:project, repository_storage: 'missing') } let(:project2) { build(:project, repository_storage: 'missing') }
......
...@@ -4,7 +4,8 @@ require 'spec_helper' ...@@ -4,7 +4,8 @@ require 'spec_helper'
RSpec.describe NamespacePolicy do RSpec.describe NamespacePolicy do
let_it_be(:parent) { create(:namespace) } let_it_be(:parent) { create(:namespace) }
let_it_be(:namespace) { create(:project_namespace, parent: parent) } let_it_be(:project) { create(:project, namespace: parent) }
let_it_be(:namespace) { project.project_namespace }
let(:permissions) do let(:permissions) do
[:owner_access, :create_projects, :admin_namespace, :read_namespace, [:owner_access, :create_projects, :admin_namespace, :read_namespace,
......
...@@ -8,7 +8,7 @@ RSpec.describe API::Namespaces do ...@@ -8,7 +8,7 @@ RSpec.describe API::Namespaces do
let_it_be(:group1) { create(:group, name: 'group.one') } let_it_be(:group1) { create(:group, name: 'group.one') }
let_it_be(:group2) { create(:group, :nested) } let_it_be(:group2) { create(:group, :nested) }
let_it_be(:project) { create(:project, namespace: group2, name: group2.name, path: group2.path) } let_it_be(:project) { create(:project, namespace: group2, name: group2.name, path: group2.path) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project_namespace) { project.project_namespace }
describe "GET /namespaces" do describe "GET /namespaces" do
context "when unauthenticated" do context "when unauthenticated" do
......
...@@ -47,7 +47,7 @@ RSpec.describe API::ProjectImport do ...@@ -47,7 +47,7 @@ RSpec.describe API::ProjectImport do
it 'executes a limited number of queries' do it 'executes a limited number of queries' do
control_count = ActiveRecord::QueryRecorder.new { subject }.count control_count = ActiveRecord::QueryRecorder.new { subject }.count
expect(control_count).to be <= 100 expect(control_count).to be <= 101
end end
it 'schedules an import using a namespace' do it 'schedules an import using a namespace' do
......
...@@ -185,9 +185,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do ...@@ -185,9 +185,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
context 'when projects have project namespaces' do context 'when projects have project namespaces' do
let_it_be(:project1) { create(:project, :private, namespace: group) } let_it_be(:project1) { create(:project, :private, namespace: group) }
let_it_be(:project_namespace1) { create(:project_namespace, project: project1) }
let_it_be(:project2) { create(:project, :private, namespace: group) } let_it_be(:project2) { create(:project, :private, namespace: group) }
let_it_be(:project_namespace2) { create(:project_namespace, project: project2) }
it_behaves_like 'project namespace path is in sync with project path' do it_behaves_like 'project namespace path is in sync with project path' do
let(:group_full_path) { "#{group.path}" } let(:group_full_path) { "#{group.path}" }
...@@ -250,21 +248,31 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do ...@@ -250,21 +248,31 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
let_it_be(:membership) { create(:group_member, :owner, group: new_parent_group, user: user) } let_it_be(:membership) { create(:group_member, :owner, group: new_parent_group, user: user) }
let_it_be(:project) { create(:project, path: 'foo', namespace: new_parent_group) } let_it_be(:project) { create(:project, path: 'foo', namespace: new_parent_group) }
before do it 'adds an error on group' do
group.update_attribute(:path, 'foo') expect(transfer_service.execute(new_parent_group)).to be_falsy
expect(transfer_service.error).to eq('Transfer failed: The parent group already has a subgroup or a project with the same path.')
end end
it 'returns false' do # currently when a project is created it gets a corresponding project namespace
expect(transfer_service.execute(new_parent_group)).to be_falsy # so we test the case where a project without a project namespace is transferred
# for backward compatibility
context 'without project namespace' do
before do
project_namespace = project.project_namespace
project.update_column(:project_namespace_id, nil)
project_namespace.delete
end end
it 'adds an error on group' do it 'adds an error on group' do
transfer_service.execute(new_parent_group) expect(project.reload.project_namespace).to be_nil
expect(transfer_service.execute(new_parent_group)).to be_falsy
expect(transfer_service.error).to eq('Transfer failed: Validation failed: Group URL has already been taken') expect(transfer_service.error).to eq('Transfer failed: Validation failed: Group URL has already been taken')
end end
end
end
context 'when projects have project namespaces' do context 'when projects have project namespaces' do
let!(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project) { create(:project, path: 'foo', namespace: new_parent_group) }
before do before do
transfer_service.execute(new_parent_group) transfer_service.execute(new_parent_group)
...@@ -275,7 +283,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do ...@@ -275,7 +283,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
let(:projects_with_project_namespace) { [project] } let(:projects_with_project_namespace) { [project] }
end end
end end
end
context 'when the group is allowed to be transferred' do context 'when the group is allowed to be transferred' do
let_it_be(:new_parent_group, reload: true) { create(:group, :public) } let_it_be(:new_parent_group, reload: true) { create(:group, :public) }
...@@ -445,8 +452,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do ...@@ -445,8 +452,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
context 'when transferring a group with project descendants' do context 'when transferring a group with project descendants' do
let!(:project1) { create(:project, :repository, :private, namespace: group) } let!(:project1) { create(:project, :repository, :private, namespace: group) }
let!(:project2) { create(:project, :repository, :internal, namespace: group) } let!(:project2) { create(:project, :repository, :internal, namespace: group) }
let!(:project_namespace1) { create(:project_namespace, project: project1) }
let!(:project_namespace2) { create(:project_namespace, project: project2) }
before do before do
TestEnv.clean_test_path TestEnv.clean_test_path
...@@ -483,8 +488,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do ...@@ -483,8 +488,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
let!(:project1) { create(:project, :repository, :public, namespace: group) } let!(:project1) { create(:project, :repository, :public, namespace: group) }
let!(:project2) { create(:project, :repository, :public, namespace: group) } let!(:project2) { create(:project, :repository, :public, namespace: group) }
let!(:new_parent_group) { create(:group, :private) } let!(:new_parent_group) { create(:group, :private) }
let!(:project_namespace1) { create(:project_namespace, project: project1) }
let!(:project_namespace2) { create(:project_namespace, project: project2) }
it 'updates projects visibility to match the new parent' do it 'updates projects visibility to match the new parent' do
group.projects.each do |project| group.projects.each do |project|
...@@ -504,8 +507,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do ...@@ -504,8 +507,6 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
let!(:project2) { create(:project, :repository, :internal, namespace: group) } let!(:project2) { create(:project, :repository, :internal, namespace: group) }
let!(:subgroup1) { create(:group, :private, parent: group) } let!(:subgroup1) { create(:group, :private, parent: group) }
let!(:subgroup2) { create(:group, :internal, parent: group) } let!(:subgroup2) { create(:group, :internal, parent: group) }
let!(:project_namespace1) { create(:project_namespace, project: project1) }
let!(:project_namespace2) { create(:project_namespace, project: project2) }
before do before do
TestEnv.clean_test_path TestEnv.clean_test_path
......
...@@ -49,6 +49,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -49,6 +49,7 @@ RSpec.describe Projects::CreateService, '#execute' do
it 'keeps them as specified' do it 'keeps them as specified' do
expect(project.name).to eq('one') expect(project.name).to eq('one')
expect(project.path).to eq('two') expect(project.path).to eq('two')
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
...@@ -58,6 +59,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -58,6 +59,7 @@ RSpec.describe Projects::CreateService, '#execute' do
it 'sets name == path' do it 'sets name == path' do
expect(project.path).to eq('one.two_three-four') expect(project.path).to eq('one.two_three-four')
expect(project.name).to eq(project.path) expect(project.name).to eq(project.path)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
...@@ -67,6 +69,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -67,6 +69,7 @@ RSpec.describe Projects::CreateService, '#execute' do
it 'sets path == name' do it 'sets path == name' do
expect(project.name).to eq('one.two_three-four') expect(project.name).to eq('one.two_three-four')
expect(project.path).to eq(project.name) expect(project.path).to eq(project.name)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
...@@ -78,6 +81,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -78,6 +81,7 @@ RSpec.describe Projects::CreateService, '#execute' do
it 'parameterizes the name' do it 'parameterizes the name' do
expect(project.name).to eq('one.two_three-four and five') expect(project.name).to eq('one.two_three-four and five')
expect(project.path).to eq('one-two_three-four-and-five') expect(project.path).to eq('one-two_three-four-and-five')
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
...@@ -111,13 +115,14 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -111,13 +115,14 @@ RSpec.describe Projects::CreateService, '#execute' do
end end
context 'user namespace' do context 'user namespace' do
it do it 'creates a project in user namespace' do
project = create_project(user, opts) project = create_project(user, opts)
expect(project).to be_valid expect(project).to be_valid
expect(project.owner).to eq(user) expect(project.owner).to eq(user)
expect(project.team.maintainers).to include(user) expect(project.team.maintainers).to include(user)
expect(project.namespace).to eq(user.namespace) expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
...@@ -151,6 +156,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -151,6 +156,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project.owner).to eq(user) expect(project.owner).to eq(user)
expect(project.team.maintainers).to contain_exactly(user) expect(project.team.maintainers).to contain_exactly(user)
expect(project.namespace).to eq(user.namespace) expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
...@@ -160,6 +166,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -160,6 +166,7 @@ RSpec.describe Projects::CreateService, '#execute' do
project = create_project(admin, opts) project = create_project(admin, opts)
expect(project).not_to be_persisted expect(project).not_to be_persisted
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
...@@ -183,6 +190,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -183,6 +190,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project.namespace).to eq(group) expect(project.namespace).to eq(group)
expect(project.team.owners).to include(user) expect(project.team.owners).to include(user)
expect(user.authorized_projects).to include(project) expect(user.authorized_projects).to include(project)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
...@@ -339,6 +347,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -339,6 +347,7 @@ RSpec.describe Projects::CreateService, '#execute' do
end end
imported_project imported_project
expect(imported_project.project_namespace).to be_in_sync_with_project(imported_project)
end end
it 'stores import data and URL' do it 'stores import data and URL' do
...@@ -406,6 +415,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -406,6 +415,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project.visibility_level).to eq(project_level) expect(project.visibility_level).to eq(project_level)
expect(project).to be_saved expect(project).to be_saved
expect(project).to be_valid expect(project).to be_valid
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
...@@ -424,6 +434,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -424,6 +434,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project.errors.messages[:visibility_level].first).to( expect(project.errors.messages[:visibility_level].first).to(
match('restricted by your GitLab administrator') match('restricted by your GitLab administrator')
) )
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
it 'does not allow a restricted visibility level for admins when admin mode is disabled' do it 'does not allow a restricted visibility level for admins when admin mode is disabled' do
...@@ -493,6 +504,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -493,6 +504,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to be_valid expect(project).to be_valid
expect(project.owner).to eq(user) expect(project.owner).to eq(user)
expect(project.namespace).to eq(user.namespace) expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
context 'when another repository already exists on disk' do context 'when another repository already exists on disk' do
...@@ -522,6 +534,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -522,6 +534,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to respond_to(:errors) expect(project).to respond_to(:errors)
expect(project.errors.messages).to have_key(:base) expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
it 'does not allow to import project when path matches existing repository on disk' do it 'does not allow to import project when path matches existing repository on disk' do
...@@ -531,6 +544,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -531,6 +544,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to respond_to(:errors) expect(project).to respond_to(:errors)
expect(project.errors.messages).to have_key(:base) expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
...@@ -555,6 +569,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -555,6 +569,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to respond_to(:errors) expect(project).to respond_to(:errors)
expect(project.errors.messages).to have_key(:base) expect(project.errors.messages).to have_key(:base)
expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk')
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
...@@ -870,6 +885,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -870,6 +885,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to be_valid expect(project).to be_valid
expect(project.shared_runners_enabled).to eq(expected_result_for_project) expect(project.shared_runners_enabled).to eq(expected_result_for_project)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
...@@ -890,6 +906,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -890,6 +906,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to be_valid expect(project).to be_valid
expect(project.shared_runners_enabled).to eq(expected_result_for_project) expect(project.shared_runners_enabled).to eq(expected_result_for_project)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
...@@ -907,6 +924,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -907,6 +924,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project.persisted?).to eq(false) expect(project.persisted?).to eq(false)
expect(project).to be_invalid expect(project).to be_invalid
expect(project.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group does not allow it') expect(project.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group does not allow it')
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
...@@ -926,6 +944,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -926,6 +944,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to be_valid expect(project).to be_valid
expect(project.shared_runners_enabled).to eq(expected_result) expect(project.shared_runners_enabled).to eq(expected_result)
expect(project.project_namespace).to be_in_sync_with_project(project)
end end
end end
end end
......
...@@ -66,8 +66,6 @@ RSpec.describe Projects::TransferService do ...@@ -66,8 +66,6 @@ RSpec.describe Projects::TransferService do
end end
context 'when project has an associated project namespace' do context 'when project has an associated project namespace' do
let!(:project_namespace) { create(:project_namespace, project: project) }
it 'keeps project namespace in sync with project' do it 'keeps project namespace in sync with project' do
transfer_result = execute_transfer transfer_result = execute_transfer
...@@ -272,8 +270,6 @@ RSpec.describe Projects::TransferService do ...@@ -272,8 +270,6 @@ RSpec.describe Projects::TransferService do
end end
context 'when project has an associated project namespace' do context 'when project has an associated project namespace' do
let!(:project_namespace) { create(:project_namespace, project: project) }
it 'keeps project namespace in sync with project' do it 'keeps project namespace in sync with project' do
attempt_project_transfer attempt_project_transfer
...@@ -294,8 +290,6 @@ RSpec.describe Projects::TransferService do ...@@ -294,8 +290,6 @@ RSpec.describe Projects::TransferService do
end end
context 'when project has an associated project namespace' do context 'when project has an associated project namespace' do
let!(:project_namespace) { create(:project_namespace, project: project) }
it 'keeps project namespace in sync with project' do it 'keeps project namespace in sync with project' do
transfer_result = execute_transfer transfer_result = execute_transfer
......
# frozen_string_literal: true
RSpec::Matchers.define :be_in_sync_with_project do |project|
match do |project_namespace|
# if project is not persisted make sure we do not have a persisted project_namespace for it
break false if project.new_record? && project_namespace&.persisted?
# don't really care if project is not in sync if the project was never persisted.
break true if project.new_record? && !project_namespace.present?
project_namespace.present? &&
project.name == project_namespace.name &&
project.path == project_namespace.path &&
project.namespace == project_namespace.parent &&
project.visibility_level == project_namespace.visibility_level &&
project.shared_runners_enabled == project_namespace.shared_runners_enabled
end
failure_message_when_negated do |project_namespace|
if project.new_record? && project_namespace&.persisted?
"expected that a non persisted project #{project} does not have a persisted project namespace #{project_namespace}"
else
<<-MSG
expected that the project's attributes name, path, namespace_id, visibility_level, shared_runners_enabled
are in sync with the corresponding project namespace attributes
MSG
end
end
end
...@@ -23,7 +23,7 @@ RSpec.shared_examples 'namespace traversal' do ...@@ -23,7 +23,7 @@ RSpec.shared_examples 'namespace traversal' do
let_it_be(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } let_it_be(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
let_it_be(:groups) { [group, nested_group, deep_nested_group, very_deep_nested_group] } let_it_be(:groups) { [group, nested_group, deep_nested_group, very_deep_nested_group] }
let_it_be(:project) { create(:project, group: nested_group) } let_it_be(:project) { create(:project, group: nested_group) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) } let_it_be(:project_namespace) { project.project_namespace }
describe '#root_ancestor' do describe '#root_ancestor' do
it 'returns the correct root ancestor' do it 'returns the correct root ancestor' 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