Commit 529a07bd authored by Bob Van Landuyt's avatar Bob Van Landuyt

Handle creating a nested group on MySQL correctly

Since we don't support nested groups on MySQL, raise an error
explaining that on import instead of trying anyway.
parent ec784b1e
...@@ -15,6 +15,10 @@ module Groups ...@@ -15,6 +15,10 @@ module Groups
return group return group
end end
if group_path.include?('/') && !Group.supports_nested_groups?
raise 'Nested groups are not supported on MySQL'
end
create_group_path create_group_path
end end
......
...@@ -2,13 +2,14 @@ require 'spec_helper' ...@@ -2,13 +2,14 @@ require 'spec_helper'
describe Gitlab::BareRepositoryImporter, repository: true do describe Gitlab::BareRepositoryImporter, repository: true do
subject(:importer) { described_class.new('default', project_path) } subject(:importer) { described_class.new('default', project_path) }
let(:project_path) { 'a-group/a-sub-group/a-project' }
let!(:admin) { create(:admin) } let!(:admin) { create(:admin) }
before do before do
allow(described_class).to receive(:log) allow(described_class).to receive(:log)
end end
shared_examples 'importing a repository' do
describe '.execute' do describe '.execute' do
it 'creates a project for a repository in storage' do it 'creates a project for a repository in storage' do
FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git")) FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git"))
...@@ -49,12 +50,10 @@ describe Gitlab::BareRepositoryImporter, repository: true do ...@@ -49,12 +50,10 @@ describe Gitlab::BareRepositoryImporter, repository: true do
end end
it 'skips importing when the project already exists' do it 'skips importing when the project already exists' do
group = create(:group, path: 'a-group') project = create(:project, path: 'a-project', namespace: existing_group)
subgroup = create(:group, path: 'a-sub-group', parent: group)
project = create(:project, path: 'a-project', namespace: subgroup)
expect(importer).not_to receive(:create_project) expect(importer).not_to receive(:create_project)
expect(importer).to receive(:log).with(" * #{project.name} (a-group/a-sub-group/a-project) exists") expect(importer).to receive(:log).with(" * #{project.name} (#{project_path}) exists")
importer.create_project_if_needed importer.create_project_if_needed
end end
...@@ -65,4 +64,37 @@ describe Gitlab::BareRepositoryImporter, repository: true do ...@@ -65,4 +64,37 @@ describe Gitlab::BareRepositoryImporter, repository: true do
expect(Project.find_by_full_path(project_path)).not_to be_nil expect(Project.find_by_full_path(project_path)).not_to be_nil
end end
end end
end
context 'with subgroups', :nested_groups do
let(:project_path) { 'a-group/a-sub-group/a-project' }
let(:existing_group) do
group = create(:group, path: 'a-group')
create(:group, path: 'a-sub-group', parent: group)
end
it_behaves_like 'importing a repository'
end
context 'without subgroups' do
let(:project_path) { 'a-group/a-project' }
let(:existing_group) { create(:group, path: 'a-group') }
it_behaves_like 'importing a repository'
end
context 'when subgroups are not available' do
let(:project_path) { 'a-group/a-sub-group/a-project' }
before do
expect(Group).to receive(:supports_nested_groups?) { false }
end
describe '#create_project_if_needed' do
it 'raises an error' do
expect { importer.create_project_if_needed }.to raise_error('Nested groups are not supported on MySQL')
end
end
end
end end
...@@ -2,10 +2,61 @@ require 'spec_helper' ...@@ -2,10 +2,61 @@ require 'spec_helper'
describe Groups::NestedCreateService do describe Groups::NestedCreateService do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:params) { { group_path: 'a-group/a-sub-group' } }
subject(:service) { described_class.new(user, params) } subject(:service) { described_class.new(user, params) }
shared_examples 'with a visibility level' do
it 'creates the group with correct visibility level' do
allow(Gitlab::CurrentSettings.current_application_settings)
.to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL }
group = service.execute
expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
end
context 'adding a visibility level ' do
it 'overwrites the visibility level' do
service = described_class.new(user, params.merge(visibility_level: Gitlab::VisibilityLevel::PRIVATE))
group = service.execute
expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
end
end
end
describe 'without subgroups' do
let(:params) { { group_path: 'a-group' } }
before do
allow(Group).to receive(:supports_nested_groups?) { false }
end
it 'creates the group' do
group = service.execute
expect(group).to be_persisted
end
it 'returns the group if it already existed' do
existing_group = create(:group, path: 'a-group')
expect(service.execute).to eq(existing_group)
end
it 'raises an error when tring to create a subgroup' do
service = described_class.new(user, group_path: 'a-group/a-sub-group')
expect { service.execute }.to raise_error('Nested groups are not supported on MySQL')
end
it_behaves_like 'with a visibility level'
end
describe 'with subgroups', :nested_groups do
let(:params) { { group_path: 'a-group/a-sub-group' } }
describe "#execute" do describe "#execute" do
it 'returns the group if it already existed' do it 'returns the group if it already existed' do
parent = create(:group, path: 'a-group', owner: user) parent = create(:group, path: 'a-group', owner: user)
...@@ -14,14 +65,14 @@ describe Groups::NestedCreateService do ...@@ -14,14 +65,14 @@ describe Groups::NestedCreateService do
expect(service.execute).to eq(child) expect(service.execute).to eq(child)
end end
it 'reuses a parent if it already existed', :nested_groups do it 'reuses a parent if it already existed' do
parent = create(:group, path: 'a-group') parent = create(:group, path: 'a-group')
parent.add_owner(user) parent.add_owner(user)
expect(service.execute.parent).to eq(parent) expect(service.execute.parent).to eq(parent)
end end
it 'creates group and subgroup in the database', :nested_groups do it 'creates group and subgroup in the database' do
service.execute service.execute
parent = Group.find_by_full_path('a-group') parent = Group.find_by_full_path('a-group')
...@@ -31,23 +82,7 @@ describe Groups::NestedCreateService do ...@@ -31,23 +82,7 @@ describe Groups::NestedCreateService do
expect(child).not_to be_nil expect(child).not_to be_nil
end end
it 'creates the group with correct visibility level' do it_behaves_like 'with a visibility level'
allow(Gitlab::CurrentSettings.current_application_settings)
.to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL }
group = service.execute
expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
end
context 'adding a visibility level ' do
let(:params) { { group_path: 'a-group/a-sub-group', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
it 'overwrites the visibility level' do
group = service.execute
expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
end
end end
end end
end end
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