Merge branch 'bvl-fix-mysql-bare-repository-importer' into 'master'

Handle creating a nested group on MySQL correctly

See merge request !13829

app/services/groups/nested_create_service.rb

@@ -15,6 +15,10 @@ module Groups
         return group
       end
 
+      if group_path.include?('/') && !Group.supports_nested_groups?
+        raise 'Nested groups are not supported on MySQL'
+      end
+
       create_group_path
     end
 

spec/lib/gitlab/bare_repository_importer_spec.rb

@@ -2,67 +2,99 @@ require 'spec_helper'
 
 describe Gitlab::BareRepositoryImporter, repository: true do
   subject(:importer) { described_class.new('default', project_path) }
-  let(:project_path) { 'a-group/a-sub-group/a-project' }
+
   let!(:admin) { create(:admin) }
 
   before do
     allow(described_class).to receive(:log)
   end
 
-  describe '.execute' do
-    it 'creates a project for a repository in storage' do
-      FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git"))
-      fake_importer = double
+  shared_examples 'importing a repository' do
+    describe '.execute' do
+      it 'creates a project for a repository in storage' do
+        FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git"))
+        fake_importer = double
 
-      expect(described_class).to receive(:new).with('default', project_path)
-                                   .and_return(fake_importer)
-      expect(fake_importer).to receive(:create_project_if_needed)
+        expect(described_class).to receive(:new).with('default', project_path)
+                                     .and_return(fake_importer)
+        expect(fake_importer).to receive(:create_project_if_needed)
 
-      described_class.execute
-    end
+        described_class.execute
+      end
 
-    it 'skips wiki repos' do
-      FileUtils.mkdir_p(File.join(TestEnv.repos_path, 'the-group', 'the-project.wiki.git'))
+      it 'skips wiki repos' do
+        FileUtils.mkdir_p(File.join(TestEnv.repos_path, 'the-group', 'the-project.wiki.git'))
 
-      expect(described_class).to receive(:log).with(' * Skipping wiki repo')
-      expect(described_class).not_to receive(:new)
+        expect(described_class).to receive(:log).with(' * Skipping wiki repo')
+        expect(described_class).not_to receive(:new)
 
-      described_class.execute
+        described_class.execute
+      end
     end
-  end
 
-  describe '#initialize' do
-    context 'without admin users' do
-      let(:admin) { nil }
+    describe '#initialize' do
+      context 'without admin users' do
+        let(:admin) { nil }
 
-      it 'raises an error' do
-        expect { importer }.to raise_error(Gitlab::BareRepositoryImporter::NoAdminError)
+        it 'raises an error' do
+          expect { importer }.to raise_error(Gitlab::BareRepositoryImporter::NoAdminError)
+        end
       end
     end
-  end
 
-  describe '#create_project_if_needed' do
-    it 'starts an import for a project that did not exist' do
-      expect(importer).to receive(:create_project)
+    describe '#create_project_if_needed' do
+      it 'starts an import for a project that did not exist' do
+        expect(importer).to receive(:create_project)
+
+        importer.create_project_if_needed
+      end
+
+      it 'skips importing when the project already exists' do
+        project = create(:project, path: 'a-project', namespace: existing_group)
+
+        expect(importer).not_to receive(:create_project)
+        expect(importer).to receive(:log).with(" * #{project.name} (#{project_path}) exists")
+
+        importer.create_project_if_needed
+      end
+
+      it 'creates a project with the correct path in the database' do
+        importer.create_project_if_needed
 
-      importer.create_project_if_needed
+        expect(Project.find_by_full_path(project_path)).not_to be_nil
+      end
     end
+  end
+
+  context 'with subgroups', :nested_groups do
+    let(:project_path) { 'a-group/a-sub-group/a-project' }
 
-    it 'skips importing when the project already exists' do
+    let(:existing_group) do
       group = create(:group, path: 'a-group')
-      subgroup = create(:group, path: 'a-sub-group', parent: group)
-      project = create(:project, path: 'a-project', namespace: subgroup)
+      create(:group, path: 'a-sub-group', parent: group)
+    end
 
-      expect(importer).not_to receive(:create_project)
-      expect(importer).to receive(:log).with(" * #{project.name} (a-group/a-sub-group/a-project) exists")
+    it_behaves_like 'importing a repository'
+  end
 
-      importer.create_project_if_needed
-    end
+  context 'without subgroups' do
+    let(:project_path) { 'a-group/a-project' }
+    let(:existing_group) { create(:group, path: 'a-group') }
 
-    it 'creates a project with the correct path in the database' do
-      importer.create_project_if_needed
+    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
 
-      expect(Project.find_by_full_path(project_path)).not_to be_nil
+    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

spec/services/groups/nested_create_service_spec.rb

@@ -2,52 +2,87 @@ require 'spec_helper'
 
 describe Groups::NestedCreateService do
   let(:user) { create(:user) }
-  let(:params) { { group_path: 'a-group/a-sub-group' } }
 
   subject(:service) { described_class.new(user, params) }
 
-  describe "#execute" do
-    it 'returns the group if it already existed' do
-      parent = create(:group, path: 'a-group', owner: user)
-      child = create(:group, path: 'a-sub-group', parent: parent, owner: user)
+  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(service.execute).to eq(child)
+      expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
     end
 
-    it 'reuses a parent if it already existed', :nested_groups do
-      parent = create(:group, path: 'a-group')
-      parent.add_owner(user)
+    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(service.execute.parent).to eq(parent)
+        expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
+      end
     end
+  end
+
+  describe 'without subgroups' do
+    let(:params) { { group_path: 'a-group' } }
 
-    it 'creates group and subgroup in the database', :nested_groups do
-      service.execute
+    before do
+      allow(Group).to receive(:supports_nested_groups?) { false }
+    end
 
-      parent = Group.find_by_full_path('a-group')
-      child = parent.children.find_by(path: 'a-sub-group')
+    it 'creates the group' do
+      group = service.execute
 
-      expect(parent).not_to be_nil
-      expect(child).not_to be_nil
+      expect(group).to be_persisted
     end
 
-    it 'creates the group with correct visibility level' do
-      allow(Gitlab::CurrentSettings.current_application_settings)
-        .to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL }
+    it 'returns the group if it already existed' do
+      existing_group = create(:group, path: 'a-group')
 
-      group = service.execute
+      expect(service.execute).to eq(existing_group)
+    end
 
-      expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
+    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
 
-    context 'adding a visibility level ' do
-      let(:params) { { group_path: 'a-group/a-sub-group', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
+    it_behaves_like 'with a visibility level'
+  end
 
-      it 'overwrites the visibility level' do
-        group = service.execute
+  describe 'with subgroups', :nested_groups do
+    let(:params) { { group_path: 'a-group/a-sub-group' } }
 
-        expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
+    describe "#execute" do
+      it 'returns the group if it already existed' do
+        parent = create(:group, path: 'a-group', owner: user)
+        child = create(:group, path: 'a-sub-group', parent: parent, owner: user)
+
+        expect(service.execute).to eq(child)
       end
+
+      it 'reuses a parent if it already existed' do
+        parent = create(:group, path: 'a-group')
+        parent.add_owner(user)
+
+        expect(service.execute.parent).to eq(parent)
+      end
+
+      it 'creates group and subgroup in the database' do
+        service.execute
+
+        parent = Group.find_by_full_path('a-group')
+        child = parent.children.find_by(path: 'a-sub-group')
+
+        expect(parent).not_to be_nil
+        expect(child).not_to be_nil
+      end
+
+      it_behaves_like 'with a visibility level'
     end
   end
 end