Merge branch 'group-import-with-ndjson' into 'master'

Introduces Group import feature with ndjson

See merge request gitlab-org/gitlab!29716

app/services/groups/import_export/import_service.rb

@@ -27,19 +27,30 @@ module Groups
       private
 
       def import_file
-        @import_file ||= Gitlab::ImportExport::FileImporter.import(importable: @group,
+        @import_file ||= Gitlab::ImportExport::FileImporter.import(
+          importable: @group,
           archive_file: nil,
-                                                                   shared: @shared)
+          shared: @shared
+        )
       end
 
       def restorer
-        @restorer ||= Gitlab::ImportExport::Group::LegacyTreeRestorer.new(
+        @restorer ||=
+          if ::Feature.enabled?(:group_import_export_ndjson, @group&.parent)
+            Gitlab::ImportExport::Group::TreeRestorer.new(
+              user: @current_user,
+              shared: @shared,
+              group: @group
+            )
+          else
+            Gitlab::ImportExport::Group::LegacyTreeRestorer.new(
               user: @current_user,
               shared: @shared,
               group: @group,
               group_hash: nil
             )
           end
+      end
 
       def remove_import_file
         upload = @group.import_export_upload

ee/spec/lib/ee/gitlab/import_export/group/tree_restorer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::ImportExport::Group::TreeRestorer do
+  include ImportExport::CommonUtil
+
+  let(:user) { create(:user) }
+  let(:group) { create(:group, name: 'group', path: 'group') }
+  let(:shared) { Gitlab::ImportExport::Shared.new(group) }
+  let(:group_tree_restorer) { described_class.new(user: user, shared: shared, group: group) }
+
+  before do
+    stub_licensed_features(board_assignee_lists: true, board_milestone_lists: true)
+
+    setup_import_export_config('group_exports/light', 'ee')
+    group.add_owner(user)
+    group_tree_restorer.restore
+  end
+
+  after do
+    cleanup_artifacts_from_extract_archive('group_exports/light', 'ee')
+  end
+
+  describe 'restore group tree' do
+    context 'epics' do
+      it 'has group epics' do
+        expect(group.epics.count).to eq(2)
+      end
+
+      it 'has award emoji' do
+        expect(group.epics.first.award_emoji.first.name).to eq('thumbsup')
+      end
+
+      it 'preserves epic state' do
+        expect(group.epics.first.state).to eq('opened')
+        expect(group.epics.last.state).to eq('closed')
+      end
+    end
+
+    context 'epic notes' do
+      it 'has epic notes' do
+        expect(group.epics.first.notes.count).to eq(1)
+      end
+
+      it 'has award emoji on epic notes' do
+        expect(group.epics.first.notes.first.award_emoji.first.name).to eq('drum')
+      end
+    end
+
+    context 'board lists' do
+      it 'has milestone & assignee lists' do
+        lists = group.boards.find_by(name: 'first board').lists
+
+        expect(lists.map(&:list_type)).to contain_exactly('assignee', 'milestone')
+      end
+    end
+
+    context 'boards' do
+      it 'has user generated milestones' do
+        board = group.boards.find_by(name: 'second board')
+
+        expect(board.milestone.title).to eq 'v4.0'
+      end
+
+      it 'does not have predefined milestones' do
+        board = group.boards.find_by(name: 'first board')
+
+        expect(board.milestone).to be_nil
+      end
+    end
+  end
+end

lib/gitlab/import_export/group/group_restorer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module ImportExport
+    module Group
+      class GroupRestorer
+        def initialize(
+          user:,
+          shared:,
+          group:,
+          attributes:,
+          importable_path:,
+          relation_reader:,
+          reader:
+        )
+          @user = user
+          @shared = shared
+          @group = group
+          @group_attributes = attributes
+          @importable_path = importable_path
+          @relation_reader = relation_reader
+          @reader = reader
+        end
+
+        def restore
+          # consume_relation returns a list of [relation, index]
+          @group_members = @relation_reader
+            .consume_relation(@importable_path, 'members')
+            .map(&:first)
+
+          return unless members_mapper.map
+
+          restorer.restore
+        end
+
+        private
+
+        def restorer
+          @relation_tree_restorer ||= RelationTreeRestorer.new(
+            user: @user,
+            shared: @shared,
+            relation_reader: @relation_reader,
+            members_mapper: members_mapper,
+            object_builder: object_builder,
+            relation_factory: relation_factory,
+            reader: @reader,
+            importable: @group,
+            importable_attributes: @group_attributes,
+            importable_path: @importable_path
+          )
+        end
+
+        def members_mapper
+          @members_mapper ||= Gitlab::ImportExport::MembersMapper.new(
+            exported_members: @group_members,
+            user: @user,
+            importable: @group
+          )
+        end
+
+        def relation_factory
+          Gitlab::ImportExport::Group::RelationFactory
+        end
+
+        def object_builder
+          Gitlab::ImportExport::Group::ObjectBuilder
+        end
+      end
+    end
+  end
+end

lib/gitlab/import_export/group/tree_restorer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module ImportExport
+    module Group
+      class TreeRestorer
+        include Gitlab::Utils::StrongMemoize
+
+        attr_reader :user, :shared
+
+        def initialize(user:, shared:, group:)
+          @user = user
+          @shared = shared
+          @top_level_group = group
+          @groups_mapping = {}
+        end
+
+        def restore
+          group_ids = relation_reader.consume_relation('groups', '_all').map { |value, _idx| Integer(value) }
+          root_group_id = group_ids.delete_at(0)
+
+          process_root(root_group_id)
+
+          group_ids.each do |group_id|
+            process_child(group_id)
+          end
+
+          true
+        rescue => e
+          shared.error(e)
+          false
+        end
+
+        class GroupAttributes
+          attr_reader :attributes, :group_id, :id, :path
+
+          def initialize(group_id, relation_reader)
+            @group_id = group_id
+
+            @path = "groups/#{group_id}"
+            @attributes = relation_reader.consume_attributes(@path)
+            @id = @attributes.delete('id')
+
+            unless @id == @group_id
+              raise ArgumentError, "Invalid group_id for #{group_id}"
+            end
+          end
+
+          def delete_attribute(name)
+            attributes.delete(name)
+          end
+
+          def delete_attributes(*names)
+            names.map(&method(:delete_attribute))
+          end
+        end
+        private_constant :GroupAttributes
+
+        private
+
+        def process_root(group_id)
+          group_attributes = GroupAttributes.new(group_id, relation_reader)
+
+          # name and path are not imported on the root group to avoid conflict
+          # with existing groups name and/or path.
+          group_attributes.delete_attributes('name', 'path')
+
+          restore_group(@top_level_group, group_attributes)
+        end
+
+        def process_child(group_id)
+          group_attributes = GroupAttributes.new(group_id, relation_reader)
+
+          group = create_group(group_attributes)
+
+          restore_group(group, group_attributes)
+        end
+
+        def create_group(group_attributes)
+          parent_id = group_attributes.delete_attribute('parent_id')
+          name = group_attributes.delete_attribute('name')
+          path = group_attributes.delete_attribute('path')
+
+          parent_group = @groups_mapping.fetch(parent_id) { raise(ArgumentError, 'Parent group not found') }
+
+          ::Groups::CreateService.new(
+            user,
+            name: name,
+            path: path,
+            parent_id: parent_group.id,
+            visibility_level: sub_group_visibility_level(group_attributes.attributes, parent_group)
+          ).execute
+        end
+
+        def restore_group(group, group_attributes)
+          @groups_mapping[group_attributes.id] = group
+
+          Group::GroupRestorer.new(
+            user: user,
+            shared: shared,
+            group: group,
+            attributes: group_attributes.attributes,
+            importable_path: group_attributes.path,
+            relation_reader: relation_reader,
+            reader: reader
+          ).restore
+        end
+
+        def relation_reader
+          strong_memoize(:relation_reader) do
+            ImportExport::JSON::NdjsonReader.new(
+              File.join(shared.export_path, 'tree')
+            )
+          end
+        end
+
+        def sub_group_visibility_level(group_hash, parent_group)
+          original_visibility_level = group_hash['visibility_level'] || Gitlab::VisibilityLevel::PRIVATE
+
+          if parent_group && parent_group.visibility_level < original_visibility_level
+            Gitlab::VisibilityLevel.closest_allowed_level(parent_group.visibility_level)
+          else
+            original_visibility_level
+          end
+        end
+
+        def reader
+          strong_memoize(:reader) do
+            Gitlab::ImportExport::Reader.new(
+              shared: @shared,
+              config: Gitlab::ImportExport::Config.new(
+                config: Gitlab::ImportExport.group_config_file
+              ).to_h
+            )
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/group/tree_restorer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::ImportExport::Group::TreeRestorer do
+  include ImportExport::CommonUtil
+
+  describe 'restore group tree' do
+    before_all do
+      # Using an admin for import, so we can check assignment of existing members
+      user = create(:admin, email: 'root@gitlabexample.com')
+      create(:user, email: 'adriene.mcclure@gitlabexample.com')
+      create(:user, email: 'gwendolyn_robel@gitlabexample.com')
+
+      RSpec::Mocks.with_temporary_scope do
+        @group = create(:group, name: 'group', path: 'group')
+        @shared = Gitlab::ImportExport::Shared.new(@group)
+
+        setup_import_export_config('group_exports/complex')
+
+        group_tree_restorer = described_class.new(user: user, shared: @shared, group: @group)
+
+        expect(group_tree_restorer.restore).to be_truthy
+      end
+    end
+
+    after(:context) do
+      cleanup_artifacts_from_extract_archive('group_exports/complex')
+    end
+
+    it 'has the group description' do
+      expect(Group.find_by_path('group').description).to eq('Group Description')
+    end
+
+    it 'has group labels' do
+      expect(@group.labels.count).to eq(10)
+    end
+
+    context 'issue boards' do
+      it 'has issue boards' do
+        expect(@group.boards.count).to eq(1)
+      end
+
+      it 'has board label lists' do
+        lists = @group.boards.find_by(name: 'first board').lists
+
+        expect(lists.count).to eq(3)
+        expect(lists.first.label.title).to eq('TSL')
+        expect(lists.second.label.title).to eq('Sosync')
+      end
+    end
+
+    it 'has badges' do
+      expect(@group.badges.count).to eq(1)
+    end
+
+    it 'has milestones' do
+      expect(@group.milestones.count).to eq(5)
+    end
+
+    it 'has group children' do
+      expect(@group.children.count).to eq(2)
+    end
+
+    it 'has group members' do
+      expect(@group.members.map(&:user).map(&:email)).to contain_exactly(
+        'root@gitlabexample.com',
+        'adriene.mcclure@gitlabexample.com',
+        'gwendolyn_robel@gitlabexample.com'
+      )
+    end
+  end
+
+  context 'child with no parent' do
+    let(:user) { create(:user) }
+    let(:group) { create(:group) }
+    let(:shared) { Gitlab::ImportExport::Shared.new(group) }
+    let(:group_tree_restorer) { described_class.new(user: user, shared: shared, group: group) }
+
+    before do
+      setup_import_export_config('group_exports/child_with_no_parent')
+
+      expect(group_tree_restorer.restore).to be_falsey
+    end
+
+    after do
+      cleanup_artifacts_from_extract_archive('group_exports/child_with_no_parent')
+    end
+
+    it 'fails when a child group does not have a valid parent_id' do
+      expect(shared.errors).to include('Parent group not found')
+    end
+  end
+
+  context 'excluded attributes' do
+    let!(:source_user) { create(:user, id: 123) }
+    let!(:importer_user) { create(:user) }
+    let(:group) { create(:group, name: 'user-inputed-name', path: 'user-inputed-path') }
+    let(:shared) { Gitlab::ImportExport::Shared.new(group) }
+    let(:group_tree_restorer) { described_class.new(user: importer_user, shared: shared, group: group) }
+    let(:exported_file) { File.join(shared.export_path, 'tree/groups/4352.json') }
+    let(:group_json) { ActiveSupport::JSON.decode(IO.read(exported_file)) }
+
+    shared_examples 'excluded attributes' do
+      excluded_attributes = %w[
+        id
+        parent_id
+        owner_id
+        created_at
+        updated_at
+        runners_token
+        runners_token_encrypted
+        saml_discovery_token
+      ]
+
+      before do
+        group.add_owner(importer_user)
+
+        setup_import_export_config('group_exports/complex')
+
+        expect(File.exist?(exported_file)).to be_truthy
+
+        group_tree_restorer.restore
+        group.reload
+      end
+
+      after do
+        cleanup_artifacts_from_extract_archive('group_exports/complex')
+      end
+
+      it 'does not import root group name' do
+        expect(group.name).to eq('user-inputed-name')
+      end
+
+      it 'does not import root group path' do
+        expect(group.path).to eq('user-inputed-path')
+      end
+
+      excluded_attributes.each do |excluded_attribute|
+        it 'does not allow override of excluded attributes' do
+          unless group.public_send(excluded_attribute).nil?
+            expect(group_json[excluded_attribute]).not_to eq(group.public_send(excluded_attribute))
+          end
+        end
+      end
+    end
+
+    include_examples 'excluded attributes'
+  end
+
+  context 'group.json file access check' do
+    let(:user) { create(:user) }
+    let!(:group) { create(:group, name: 'group2', path: 'group2') }
+    let(:shared) { Gitlab::ImportExport::Shared.new(group) }
+    let(:group_tree_restorer) { described_class.new(user: user, shared: shared, group: group) }
+
+    it 'does not read a symlink' do
+      Dir.mktmpdir do |tmpdir|
+        FileUtils.mkdir_p(File.join(tmpdir, 'tree', 'groups'))
+        setup_symlink(tmpdir, 'tree/groups/_all.ndjson')
+
+        allow(shared).to receive(:export_path).and_return(tmpdir)
+
+        expect(group_tree_restorer.restore).to eq(false)
+        expect(shared.errors).to include('Incorrect JSON format')
+      end
+    end
+  end
+
+  context 'group visibility levels' do
+    let(:user) { create(:user) }
+    let(:shared) { Gitlab::ImportExport::Shared.new(group) }
+    let(:group_tree_restorer) { described_class.new(user: user, shared: shared, group: group) }
+
+    before do
+      setup_import_export_config(filepath)
+
+      group_tree_restorer.restore
+    end
+
+    after do
+      cleanup_artifacts_from_extract_archive(filepath)
+    end
+
+    shared_examples 'with visibility level' do |visibility_level, expected_visibilities|
+      context "when visibility level is #{visibility_level}" do
+        let(:group) { create(:group, visibility_level) }
+        let(:filepath) { "group_exports/visibility_levels/#{visibility_level}" }
+
+        it "imports all subgroups as #{visibility_level}" do
+          expect(group.children.map(&:visibility_level)).to eq(expected_visibilities)
+        end
+      end
+    end
+
+    include_examples 'with visibility level', :public, [20, 10, 0]
+    include_examples 'with visibility level', :private, [0, 0, 0]
+    include_examples 'with visibility level', :internal, [10, 10, 0]
+  end
+end

spec/services/groups/import_export/import_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Groups::ImportExport::ImportService do
-  describe '#execute' do
+  describe '#execute with TreeRestorer' do
     let(:user) { create(:admin) }
     let(:group) { create(:group) }
     let(:service) { described_class.new(group: group, user: user) }
@@ -14,6 +14,8 @@ describe Groups::ImportExport::ImportService do
     subject { service.execute }
 
     before do
+      stub_feature_flags(group_import_export_ndjson: true)
+
       ImportExportUpload.create(group: group, import_file: import_file)
 
       allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
@@ -102,4 +104,106 @@ describe Groups::ImportExport::ImportService do
       end
     end
   end
+
+  describe '#execute with LegacyTreeRestorer' do
+    let(:user) { create(:admin) }
+    let(:group) { create(:group) }
+    let(:service) { described_class.new(group: group, user: user) }
+    let(:import_file) { fixture_file_upload('spec/fixtures/legacy_group_export.tar.gz') }
+
+    let(:import_logger) { instance_double(Gitlab::Import::Logger) }
+
+    subject { service.execute }
+
+    before do
+      stub_feature_flags(group_import_export_ndjson: false)
+
+      ImportExportUpload.create(group: group, import_file: import_file)
+
+      allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
+      allow(import_logger).to receive(:error)
+      allow(import_logger).to receive(:info)
+    end
+
+    context 'when user has correct permissions' do
+      it 'imports group structure successfully' do
+        expect(subject).to be_truthy
+      end
+
+      it 'removes import file' do
+        subject
+
+        expect(group.import_export_upload.import_file.file).to be_nil
+      end
+
+      it 'logs the import success' do
+        expect(import_logger).to receive(:info).with(
+          group_id:   group.id,
+          group_name: group.name,
+          message:    'Group Import/Export: Import succeeded'
+        ).once
+
+        subject
+      end
+    end
+
+    context 'when user does not have correct permissions' do
+      let(:user) { create(:user) }
+
+      it 'logs the error and raises an exception' do
+        expect(import_logger).to receive(:error).with(
+          group_id:   group.id,
+          group_name: group.name,
+          message:    a_string_including('Errors occurred')
+        )
+
+        expect { subject }.to raise_error(Gitlab::ImportExport::Error)
+      end
+
+      it 'tracks the error' do
+        shared = Gitlab::ImportExport::Shared.new(group)
+        allow(Gitlab::ImportExport::Shared).to receive(:new).and_return(shared)
+
+        expect(shared).to receive(:error) do |param|
+          expect(param.message).to include 'does not have required permissions for'
+        end
+
+        expect { subject }.to raise_error(Gitlab::ImportExport::Error)
+      end
+    end
+
+    context 'when there are errors with the import file' do
+      let(:import_file) { fixture_file_upload('spec/fixtures/legacy_symlink_export.tar.gz') }
+
+      it 'logs the error and raises an exception' do
+        expect(import_logger).to receive(:error).with(
+          group_id:   group.id,
+          group_name: group.name,
+          message:    a_string_including('Errors occurred')
+        ).once
+
+        expect { subject }.to raise_error(Gitlab::ImportExport::Error)
+      end
+    end
+
+    context 'when there are errors with the sub-relations' do
+      let(:import_file) { fixture_file_upload('spec/fixtures/legacy_group_export_invalid_subrelations.tar.gz') }
+
+      it 'successfully imports the group' do
+        expect(subject).to be_truthy
+      end
+
+      it 'logs the import success' do
+        allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
+
+        expect(import_logger).to receive(:info).with(
+          group_id:   group.id,
+          group_name: group.name,
+          message:    'Group Import/Export: Import succeeded'
+        )
+
+        subject
+      end
+    end
+  end
 end