diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-09-05 16:51:27 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-09-06 15:21:17 +0300 |
commit | 0eeadb2dd2cf20ab1c12f1b0e86fa1227e044b41 (patch) | |
tree | 8ac0ea9a3f87259fe33df643c4db5902b0c79ac5 /spec/lib/gitlab | |
parent | 5857f0e57aa17c93ac871f08da7dbc7a4abe9b9f (diff) |
Normalize import_export structure
This brings a significant refactor to how
we handle `import_export.yml`, merge it with EE
and how we handle that for reader and saver.
This is meant to simplify the code, and remove
a ton of conditions to handle different models
of the structure.
This is also meant to prepare the structure
to extend it much easier, like adding `preload:`
or additional object types when needed.
This does not change the behavior of import/export,
rather unifies and simplifies the current implementation.
Diffstat (limited to 'spec/lib/gitlab')
6 files changed, 370 insertions, 227 deletions
diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb index fef84c87509..cc8ca1d87e3 100644 --- a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb @@ -12,7 +12,7 @@ describe 'Import/Export attribute configuration' do let(:config_hash) { Gitlab::ImportExport::Config.new.to_h.deep_stringify_keys } let(:relation_names) do - names = names_from_tree(config_hash['project_tree']) + names = names_from_tree(config_hash.dig('tree', 'project')) # Remove duplicated or add missing models # - project is not part of the tree, so it has to be added manually. diff --git a/spec/lib/gitlab/import_export/attributes_finder_spec.rb b/spec/lib/gitlab/import_export/attributes_finder_spec.rb new file mode 100644 index 00000000000..208b60844e3 --- /dev/null +++ b/spec/lib/gitlab/import_export/attributes_finder_spec.rb @@ -0,0 +1,195 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::ImportExport::AttributesFinder do + describe '#find_root' do + subject { described_class.new(config: config).find_root(model_key) } + + let(:test_config) { 'spec/support/import_export/import_export.yml' } + let(:config) { Gitlab::ImportExport::Config.new.to_h } + let(:model_key) { :project } + + let(:project_tree_hash) do + { + except: [:id, :created_at], + include: [ + { issues: { include: [] } }, + { labels: { include: [] } }, + { merge_requests: { + except: [:iid], + include: [ + { merge_request_diff: { + include: [] + } }, + { merge_request_test: { include: [] } } + ], + only: [:id] + } }, + { commit_statuses: { + include: [{ commit: { include: [] } }] + } }, + { project_members: { + include: [{ user: { include: [], + only: [:email] } }] + } } + ] + } + end + + before do + allow_any_instance_of(Gitlab::ImportExport).to receive(:config_file).and_return(test_config) + end + + it 'generates hash from project tree config' do + is_expected.to match(project_tree_hash) + end + + context 'individual scenarios' do + it 'generates the correct hash for a single project relation' do + setup_yaml(tree: { project: [:issues] }) + + is_expected.to match( + include: [{ issues: { include: [] } }] + ) + end + + it 'generates the correct hash for a single project feature relation' do + setup_yaml(tree: { project: [:project_feature] }) + + is_expected.to match( + include: [{ project_feature: { include: [] } }] + ) + end + + it 'generates the correct hash for a multiple project relation' do + setup_yaml(tree: { project: [:issues, :snippets] }) + + is_expected.to match( + include: [{ issues: { include: [] } }, + { snippets: { include: [] } }] + ) + end + + it 'generates the correct hash for a single sub-relation' do + setup_yaml(tree: { project: [issues: [:notes]] }) + + is_expected.to match( + include: [{ issues: { include: [{ notes: { include: [] } }] } }] + ) + end + + it 'generates the correct hash for a multiple sub-relation' do + setup_yaml(tree: { project: [merge_requests: [:notes, :merge_request_diff]] }) + + is_expected.to match( + include: [{ merge_requests: + { include: [{ notes: { include: [] } }, + { merge_request_diff: { include: [] } }] } }] + ) + end + + it 'generates the correct hash for a sub-relation with another sub-relation' do + setup_yaml(tree: { project: [merge_requests: [notes: [:author]]] }) + + is_expected.to match( + include: [{ merge_requests: { + include: [{ notes: { include: [{ author: { include: [] } }] } }] + } }] + ) + end + + it 'generates the correct hash for a relation with included attributes' do + setup_yaml(tree: { project: [:issues] }, + included_attributes: { issues: [:name, :description] }) + + is_expected.to match( + include: [{ issues: { include: [], + only: [:name, :description] } }] + ) + end + + it 'generates the correct hash for a relation with excluded attributes' do + setup_yaml(tree: { project: [:issues] }, + excluded_attributes: { issues: [:name] }) + + is_expected.to match( + include: [{ issues: { except: [:name], + include: [] } }] + ) + end + + it 'generates the correct hash for a relation with both excluded and included attributes' do + setup_yaml(tree: { project: [:issues] }, + excluded_attributes: { issues: [:name] }, + included_attributes: { issues: [:description] }) + + is_expected.to match( + include: [{ issues: { except: [:name], + include: [], + only: [:description] } }] + ) + end + + it 'generates the correct hash for a relation with custom methods' do + setup_yaml(tree: { project: [:issues] }, + methods: { issues: [:name] }) + + is_expected.to match( + include: [{ issues: { include: [], + methods: [:name] } }] + ) + end + + def setup_yaml(hash) + allow(YAML).to receive(:load_file).with(test_config).and_return(hash) + end + end + end + + describe '#find_relations_tree' do + subject { described_class.new(config: config).find_relations_tree(model_key) } + + let(:tree) { { project: { issues: {} } } } + let(:model_key) { :project } + + context 'when initialized with config including tree' do + let(:config) { { tree: tree } } + + context 'when relation is in top-level keys of the tree' do + it { is_expected.to eq({ issues: {} }) } + end + + context 'when the relation is not in top-level keys' do + let(:model_key) { :issues } + + it { is_expected.to be_nil } + end + end + + context 'when tree is not present in config' do + let(:config) { {} } + + it { is_expected.to be_nil } + end + end + + describe '#find_excluded_keys' do + subject { described_class.new(config: config).find_excluded_keys(klass_name) } + + let(:klass_name) { 'project' } + + context 'when initialized with excluded_attributes' do + let(:config) { { excluded_attributes: excluded_attributes } } + let(:excluded_attributes) { { project: [:name, :path], issues: [:milestone_id] } } + + it { is_expected.to eq(%w[name path]) } + end + + context 'when excluded_attributes are not present in config' do + let(:config) { {} } + + it { is_expected.to eq([]) } + end + end +end diff --git a/spec/lib/gitlab/import_export/config_spec.rb b/spec/lib/gitlab/import_export/config_spec.rb index cf396dba382..e53db37def4 100644 --- a/spec/lib/gitlab/import_export/config_spec.rb +++ b/spec/lib/gitlab/import_export/config_spec.rb @@ -1,163 +1,159 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' +require 'rspec-parameterized' describe Gitlab::ImportExport::Config do let(:yaml_file) { described_class.new } describe '#to_h' do - context 'when using CE' do - before do - allow(yaml_file) - .to receive(:merge?) - .and_return(false) + subject { yaml_file.to_h } + + context 'when using default config' do + using RSpec::Parameterized::TableSyntax + + where(:ee) do + [true, false] end - it 'just returns the parsed Hash without the EE section' do - expected = YAML.load_file(Gitlab::ImportExport.config_file) - expected.delete('ee') + with_them do + before do + allow(Gitlab).to receive(:ee?) { ee } + end - expect(yaml_file.to_h).to eq(expected) + it 'parses default config' do + expect { subject }.not_to raise_error + expect(subject).to be_a(Hash) + expect(subject.keys).to contain_exactly( + :tree, :excluded_attributes, :included_attributes, :methods) + end end end - context 'when using EE' do - before do - allow(yaml_file) - .to receive(:merge?) - .and_return(true) - end + context 'when using custom config' do + let(:config) do + <<-EOF.strip_heredoc + tree: + project: + - labels: + - :priorities + - milestones: + - events: + - :push_event_payload - it 'merges the EE project tree into the CE project tree' do - allow(yaml_file) - .to receive(:parse_yaml) - .and_return({ - 'project_tree' => [ - { - 'issues' => [ - :id, - :title, - { 'notes' => [:id, :note, { 'author' => [:name] }] } - ] - } - ], - 'ee' => { - 'project_tree' => [ - { - 'issues' => [ - :description, - { 'notes' => [:date, { 'author' => [:email] }] } - ] - }, - { 'foo' => [{ 'bar' => %i[baz] }] } - ] - } - }) + included_attributes: + user: + - :id - expect(yaml_file.to_h).to eq({ - 'project_tree' => [ - { - 'issues' => [ - :id, - :title, - { - 'notes' => [ - :id, - :note, - { 'author' => [:name, :email] }, - :date - ] - }, - :description - ] - }, - { 'foo' => [{ 'bar' => %i[baz] }] } - ] - }) + excluded_attributes: + project: + - :name + + methods: + labels: + - :type + events: + - :action + + ee: + tree: + project: + protected_branches: + - :unprotect_access_levels + included_attributes: + user: + - :name_ee + excluded_attributes: + project: + - :name_without_ee + methods: + labels: + - :type_ee + events_ee: + - :action_ee + EOF end - it 'merges the excluded attributes list' do - allow(yaml_file) - .to receive(:parse_yaml) - .and_return({ - 'project_tree' => [], - 'excluded_attributes' => { - 'project' => %i[id title], - 'notes' => %i[id] - }, - 'ee' => { - 'project_tree' => [], - 'excluded_attributes' => { - 'project' => %i[date], - 'foo' => %i[bar baz] - } - } - }) - - expect(yaml_file.to_h).to eq({ - 'project_tree' => [], - 'excluded_attributes' => { - 'project' => %i[id title date], - 'notes' => %i[id], - 'foo' => %i[bar baz] - } - }) + let(:config_hash) { YAML.safe_load(config, [Symbol]) } + + before do + allow_any_instance_of(described_class).to receive(:parse_yaml) do + config_hash.deep_dup + end end - it 'merges the included attributes list' do - allow(yaml_file) - .to receive(:parse_yaml) - .and_return({ - 'project_tree' => [], - 'included_attributes' => { - 'project' => %i[id title], - 'notes' => %i[id] - }, - 'ee' => { - 'project_tree' => [], - 'included_attributes' => { - 'project' => %i[date], - 'foo' => %i[bar baz] + context 'when using CE' do + before do + allow(Gitlab).to receive(:ee?) { false } + end + + it 'just returns the normalized Hash' do + is_expected.to eq( + { + tree: { + project: { + labels: { + priorities: {} + }, + milestones: { + events: { + push_event_payload: {} + } + } + } + }, + included_attributes: { + user: [:id] + }, + excluded_attributes: { + project: [:name] + }, + methods: { + labels: [:type], + events: [:action] } } - }) - - expect(yaml_file.to_h).to eq({ - 'project_tree' => [], - 'included_attributes' => { - 'project' => %i[id title date], - 'notes' => %i[id], - 'foo' => %i[bar baz] - } - }) + ) + end end - it 'merges the methods list' do - allow(yaml_file) - .to receive(:parse_yaml) - .and_return({ - 'project_tree' => [], - 'methods' => { - 'project' => %i[id title], - 'notes' => %i[id] - }, - 'ee' => { - 'project_tree' => [], - 'methods' => { - 'project' => %i[date], - 'foo' => %i[bar baz] + context 'when using EE' do + before do + allow(Gitlab).to receive(:ee?) { true } + end + + it 'just returns the normalized Hash' do + is_expected.to eq( + { + tree: { + project: { + labels: { + priorities: {} + }, + milestones: { + events: { + push_event_payload: {} + } + }, + protected_branches: { + unprotect_access_levels: {} + } + } + }, + included_attributes: { + user: [:id, :name_ee] + }, + excluded_attributes: { + project: [:name, :name_without_ee] + }, + methods: { + labels: [:type, :type_ee], + events: [:action], + events_ee: [:action_ee] } } - }) - - expect(yaml_file.to_h).to eq({ - 'project_tree' => [], - 'methods' => { - 'project' => %i[id title date], - 'notes' => %i[id], - 'foo' => %i[bar baz] - } - }) + ) + end end end end diff --git a/spec/lib/gitlab/import_export/model_configuration_spec.rb b/spec/lib/gitlab/import_export/model_configuration_spec.rb index 5ed9fef1597..3442e22c11f 100644 --- a/spec/lib/gitlab/import_export/model_configuration_spec.rb +++ b/spec/lib/gitlab/import_export/model_configuration_spec.rb @@ -8,7 +8,7 @@ describe 'Import/Export model configuration' do let(:config_hash) { Gitlab::ImportExport::Config.new.to_h.deep_stringify_keys } let(:model_names) do - names = names_from_tree(config_hash['project_tree']) + names = names_from_tree(config_hash.dig('tree', 'project')) # Remove duplicated or add missing models # - project is not part of the tree, so it has to be added manually. diff --git a/spec/lib/gitlab/import_export/reader_spec.rb b/spec/lib/gitlab/import_export/reader_spec.rb index f93ff074770..87f665bd995 100644 --- a/spec/lib/gitlab/import_export/reader_spec.rb +++ b/spec/lib/gitlab/import_export/reader_spec.rb @@ -2,96 +2,45 @@ require 'spec_helper' describe Gitlab::ImportExport::Reader do let(:shared) { Gitlab::ImportExport::Shared.new(nil) } - let(:test_config) { 'spec/support/import_export/import_export.yml' } - let(:project_tree_hash) do - { - except: [:id, :created_at], - include: [:issues, :labels, - { merge_requests: { - only: [:id], - except: [:iid], - include: [:merge_request_diff, :merge_request_test] - } }, - { commit_statuses: { include: :commit } }, - { project_members: { include: { user: { only: [:email] } } } }] - } - end - - before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:config_file).and_return(test_config) - end - - it 'generates hash from project tree config' do - expect(described_class.new(shared: shared).project_tree).to match(project_tree_hash) - end - - context 'individual scenarios' do - it 'generates the correct hash for a single project relation' do - setup_yaml(project_tree: [:issues]) - - expect(described_class.new(shared: shared).project_tree).to match(include: [:issues]) - end - - it 'generates the correct hash for a single project feature relation' do - setup_yaml(project_tree: [:project_feature]) - expect(described_class.new(shared: shared).project_tree).to match(include: [:project_feature]) - end + describe '#project_tree' do + subject { described_class.new(shared: shared).project_tree } - it 'generates the correct hash for a multiple project relation' do - setup_yaml(project_tree: [:issues, :snippets]) + it 'delegates to AttributesFinder#find_root' do + expect_any_instance_of(Gitlab::ImportExport::AttributesFinder) + .to receive(:find_root) + .with(:project) - expect(described_class.new(shared: shared).project_tree).to match(include: [:issues, :snippets]) + subject end - it 'generates the correct hash for a single sub-relation' do - setup_yaml(project_tree: [issues: [:notes]]) + context 'when exception raised' do + before do + expect_any_instance_of(Gitlab::ImportExport::AttributesFinder) + .to receive(:find_root) + .with(:project) + .and_raise(StandardError) + end - expect(described_class.new(shared: shared).project_tree).to match(include: [{ issues: { include: :notes } }]) - end - - it 'generates the correct hash for a multiple sub-relation' do - setup_yaml(project_tree: [merge_requests: [:notes, :merge_request_diff]]) - - expect(described_class.new(shared: shared).project_tree).to match(include: [{ merge_requests: { include: [:notes, :merge_request_diff] } }]) - end + it { is_expected.to be false } - it 'generates the correct hash for a sub-relation with another sub-relation' do - setup_yaml(project_tree: [merge_requests: [notes: :author]]) + it 'logs the error' do + expect(shared).to receive(:error).with(instance_of(StandardError)) - expect(described_class.new(shared: shared).project_tree).to match(include: [{ merge_requests: { include: { notes: { include: :author } } } }]) + subject + end end + end - it 'generates the correct hash for a relation with included attributes' do - setup_yaml(project_tree: [:issues], included_attributes: { issues: [:name, :description] }) - - expect(described_class.new(shared: shared).project_tree).to match(include: [{ issues: { only: [:name, :description] } }]) - end - - it 'generates the correct hash for a relation with excluded attributes' do - setup_yaml(project_tree: [:issues], excluded_attributes: { issues: [:name] }) - - expect(described_class.new(shared: shared).project_tree).to match(include: [{ issues: { except: [:name] } }]) - end - - it 'generates the correct hash for a relation with both excluded and included attributes' do - setup_yaml(project_tree: [:issues], excluded_attributes: { issues: [:name] }, included_attributes: { issues: [:description] }) - - expect(described_class.new(shared: shared).project_tree).to match(include: [{ issues: { except: [:name], only: [:description] } }]) - end - - it 'generates the correct hash for a relation with custom methods' do - setup_yaml(project_tree: [:issues], methods: { issues: [:name] }) - - expect(described_class.new(shared: shared).project_tree).to match(include: [{ issues: { methods: [:name] } }]) - end + describe '#group_members_tree' do + subject { described_class.new(shared: shared).group_members_tree } - it 'generates the correct hash for group members' do - expect(described_class.new(shared: shared).group_members_tree).to match({ include: { user: { only: [:email] } } }) - end + it 'delegates to AttributesFinder#find_root' do + expect_any_instance_of(Gitlab::ImportExport::AttributesFinder) + .to receive(:find_root) + .with(:group_members) - def setup_yaml(hash) - allow(YAML).to receive(:load_file).with(test_config).and_return(hash) + subject end end end diff --git a/spec/lib/gitlab/import_export/relation_rename_service_spec.rb b/spec/lib/gitlab/import_export/relation_rename_service_spec.rb index 15748407f0c..17bb5bcc155 100644 --- a/spec/lib/gitlab/import_export/relation_rename_service_spec.rb +++ b/spec/lib/gitlab/import_export/relation_rename_service_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::ImportExport::RelationRenameService do let(:user) { create(:admin) } let(:group) { create(:group, :nested) } - let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') } + let!(:project) { create(:project, :builds_disabled, :issues_disabled, group: group, name: 'project', path: 'project') } let(:shared) { project.import_export_shared } before do @@ -24,7 +24,6 @@ describe Gitlab::ImportExport::RelationRenameService do let(:import_path) { 'spec/lib/gitlab/import_export' } let(:file_content) { IO.read("#{import_path}/project.json") } let!(:json_file) { ActiveSupport::JSON.decode(file_content) } - let(:tree_hash) { project_tree_restorer.instance_variable_get(:@tree_hash) } before do allow(shared).to receive(:export_path).and_return(import_path) @@ -92,21 +91,25 @@ describe Gitlab::ImportExport::RelationRenameService do end context 'when exporting' do - let(:project_tree_saver) { Gitlab::ImportExport::ProjectTreeSaver.new(project: project, current_user: user, shared: shared) } - let(:project_tree) { project_tree_saver.send(:project_json) } + let(:export_content_path) { project_tree_saver.full_path } + let(:export_content_hash) { ActiveSupport::JSON.decode(File.read(export_content_path)) } + let(:injected_hash) { renames.values.product([{}]).to_h } - it 'adds old relationships to the exported file' do - project_tree.merge!(renames.values.map { |new_name| [new_name, []] }.to_h) + let(:project_tree_saver) do + Gitlab::ImportExport::ProjectTreeSaver.new( + project: project, current_user: user, shared: shared) + end - allow(project_tree_saver).to receive(:save) do |arg| - project_tree_saver.send(:project_json_tree) + it 'adds old relationships to the exported file' do + # we inject relations with new names that should be rewritten + expect(project_tree_saver).to receive(:serialize_project_tree).and_wrap_original do |method, *args| + method.call(*args).merge(injected_hash) end - result = project_tree_saver.save - - saved_data = ActiveSupport::JSON.decode(result) + expect(project_tree_saver.save).to eq(true) - expect(saved_data.keys).to include(*(renames.keys + renames.values)) + expect(export_content_hash.keys).to include(*renames.keys) + expect(export_content_hash.keys).to include(*renames.values) end end end |