diff options
Diffstat (limited to 'spec/lib/bulk_imports')
-rw-r--r-- | spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb | 7 | ||||
-rw-r--r-- | spec/lib/bulk_imports/common/transformers/graphql_cleaner_transformer_spec.rb | 88 | ||||
-rw-r--r-- | spec/lib/bulk_imports/common/transformers/hash_key_digger_spec.rb | 28 | ||||
-rw-r--r-- | spec/lib/bulk_imports/common/transformers/prohibited_attributes_transformer_spec.rb | 72 | ||||
-rw-r--r-- | spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb | 11 | ||||
-rw-r--r-- | spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb | 5 | ||||
-rw-r--r-- | spec/lib/bulk_imports/importers/group_importer_spec.rb | 30 | ||||
-rw-r--r-- | spec/lib/bulk_imports/pipeline/runner_spec.rb | 169 | ||||
-rw-r--r-- | spec/lib/bulk_imports/pipeline_spec.rb (renamed from spec/lib/bulk_imports/pipeline/attributes_spec.rb) | 10 |
9 files changed, 266 insertions, 154 deletions
diff --git a/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb b/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb index cde8e2d5c18..a7a19fb73fc 100644 --- a/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb +++ b/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb @@ -41,12 +41,11 @@ RSpec.describe BulkImports::Common::Extractors::GraphqlExtractor do end context 'when variables are present' do - let(:query) { { query: double(to_s: 'test', variables: { full_path: :source_full_path }) } } + let(:variables) { { foo: :bar } } + let(:query) { { query: double(to_s: 'test', variables: variables) } } it 'builds graphql query variables for import entity' do - expected_variables = { full_path: import_entity.source_full_path } - - expect(graphql_client).to receive(:execute).with(anything, expected_variables) + expect(graphql_client).to receive(:execute).with(anything, variables) subject.extract(context).first end diff --git a/spec/lib/bulk_imports/common/transformers/graphql_cleaner_transformer_spec.rb b/spec/lib/bulk_imports/common/transformers/graphql_cleaner_transformer_spec.rb deleted file mode 100644 index 8f39b6e7c93..00000000000 --- a/spec/lib/bulk_imports/common/transformers/graphql_cleaner_transformer_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe BulkImports::Common::Transformers::GraphqlCleanerTransformer do - describe '#transform' do - let_it_be(:expected_output) do - { - 'name' => 'test', - 'fullName' => 'test', - 'description' => 'test', - 'labels' => [ - { 'title' => 'label1' }, - { 'title' => 'label2' }, - { 'title' => 'label3' } - ] - } - end - - it 'deep cleans hash from GraphQL keys' do - data = { - 'data' => { - 'group' => { - 'name' => 'test', - 'fullName' => 'test', - 'description' => 'test', - 'labels' => { - 'edges' => [ - { 'node' => { 'title' => 'label1' } }, - { 'node' => { 'title' => 'label2' } }, - { 'node' => { 'title' => 'label3' } } - ] - } - } - } - } - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq(expected_output) - end - - context 'when data does not have data/group nesting' do - it 'deep cleans hash from GraphQL keys' do - data = { - 'name' => 'test', - 'fullName' => 'test', - 'description' => 'test', - 'labels' => { - 'edges' => [ - { 'node' => { 'title' => 'label1' } }, - { 'node' => { 'title' => 'label2' } }, - { 'node' => { 'title' => 'label3' } } - ] - } - } - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq(expected_output) - end - end - - context 'when data is not a hash' do - it 'does not perform transformation' do - data = 'test' - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq(data) - end - end - - context 'when nested data is not an array or hash' do - it 'only removes top level data/group keys' do - data = { - 'data' => { - 'group' => 'test' - } - } - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq('test') - end - end - end -end diff --git a/spec/lib/bulk_imports/common/transformers/hash_key_digger_spec.rb b/spec/lib/bulk_imports/common/transformers/hash_key_digger_spec.rb new file mode 100644 index 00000000000..2b33701653e --- /dev/null +++ b/spec/lib/bulk_imports/common/transformers/hash_key_digger_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Common::Transformers::HashKeyDigger do + describe '#transform' do + it 'when the key_path is an array' do + data = { foo: { bar: :value } } + key_path = %i[foo bar] + transformed = described_class.new(key_path: key_path).transform(nil, data) + + expect(transformed).to eq(:value) + end + + it 'when the key_path is not an array' do + data = { foo: { bar: :value } } + key_path = :foo + transformed = described_class.new(key_path: key_path).transform(nil, data) + + expect(transformed).to eq({ bar: :value }) + end + + it "when the data is not a hash" do + expect { described_class.new(key_path: nil).transform(nil, nil) } + .to raise_error(ArgumentError, "Given data must be a Hash") + end + end +end diff --git a/spec/lib/bulk_imports/common/transformers/prohibited_attributes_transformer_spec.rb b/spec/lib/bulk_imports/common/transformers/prohibited_attributes_transformer_spec.rb new file mode 100644 index 00000000000..03d138b227c --- /dev/null +++ b/spec/lib/bulk_imports/common/transformers/prohibited_attributes_transformer_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Common::Transformers::ProhibitedAttributesTransformer do + describe '#transform' do + let_it_be(:hash) do + { + 'id' => 101, + 'service_id' => 99, + 'moved_to_id' => 99, + 'namespace_id' => 99, + 'ci_id' => 99, + 'random_project_id' => 99, + 'random_id' => 99, + 'milestone_id' => 99, + 'project_id' => 99, + 'user_id' => 99, + 'random_id_in_the_middle' => 99, + 'notid' => 99, + 'import_source' => 'test', + 'import_type' => 'test', + 'non_existent_attr' => 'test', + 'some_html' => '<p>dodgy html</p>', + 'legit_html' => '<p>legit html</p>', + '_html' => '<p>perfectly ordinary html</p>', + 'cached_markdown_version' => 12345, + 'custom_attributes' => 'test', + 'some_attributes_metadata' => 'test', + 'group_id' => 99, + 'commit_id' => 99, + 'issue_ids' => [1, 2, 3], + 'merge_request_ids' => [1, 2, 3], + 'note_ids' => [1, 2, 3], + 'remote_attachment_url' => 'http://something.dodgy', + 'remote_attachment_request_header' => 'bad value', + 'remote_attachment_urls' => %w(http://something.dodgy http://something.okay), + 'attributes' => { + 'issue_ids' => [1, 2, 3], + 'merge_request_ids' => [1, 2, 3], + 'note_ids' => [1, 2, 3] + }, + 'variables_attributes' => { + 'id' => 1 + }, + 'attr_with_nested_attrs' => { + 'nested_id' => 1, + 'nested_attr' => 2 + } + } + end + + let(:expected_hash) do + { + 'random_id_in_the_middle' => 99, + 'notid' => 99, + 'import_source' => 'test', + 'import_type' => 'test', + 'non_existent_attr' => 'test', + 'attr_with_nested_attrs' => { + 'nested_attr' => 2 + } + } + end + + it 'removes prohibited attributes' do + transformed_hash = subject.transform(nil, hash) + + expect(transformed_hash).to eq(expected_hash) + end + end +end diff --git a/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb b/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb index 3949dd23b49..c9b481388c3 100644 --- a/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb +++ b/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb @@ -72,7 +72,6 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do describe 'pipeline parts' do it { expect(described_class).to include_module(BulkImports::Pipeline) } - it { expect(described_class).to include_module(BulkImports::Pipeline::Attributes) } it { expect(described_class).to include_module(BulkImports::Pipeline::Runner) } it 'has extractors' do @@ -90,13 +89,17 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do it 'has transformers' do expect(described_class.transformers) .to contain_exactly( - { klass: BulkImports::Common::Transformers::GraphqlCleanerTransformer, options: nil }, + { klass: BulkImports::Common::Transformers::HashKeyDigger, options: { key_path: %w[data group] } }, { klass: BulkImports::Common::Transformers::UnderscorifyKeysTransformer, options: nil }, - { klass: BulkImports::Groups::Transformers::GroupAttributesTransformer, options: nil }) + { klass: BulkImports::Common::Transformers::ProhibitedAttributesTransformer, options: nil }, + { klass: BulkImports::Groups::Transformers::GroupAttributesTransformer, options: nil } + ) end it 'has loaders' do - expect(described_class.loaders).to contain_exactly({ klass: BulkImports::Groups::Loaders::GroupLoader, options: nil }) + expect(described_class.loaders).to contain_exactly({ + klass: BulkImports::Groups::Loaders::GroupLoader, options: nil + }) end end end diff --git a/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb b/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb index 60a4a796682..788a6e98c45 100644 --- a/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb +++ b/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb @@ -55,7 +55,6 @@ RSpec.describe BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline do describe 'pipeline parts' do it { expect(described_class).to include_module(BulkImports::Pipeline) } - it { expect(described_class).to include_module(BulkImports::Pipeline::Attributes) } it { expect(described_class).to include_module(BulkImports::Pipeline::Runner) } it 'has extractors' do @@ -67,8 +66,8 @@ RSpec.describe BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline do it 'has transformers' do expect(described_class.transformers).to contain_exactly( - klass: BulkImports::Groups::Transformers::SubgroupToEntityTransformer, - options: nil + { klass: BulkImports::Common::Transformers::ProhibitedAttributesTransformer, options: nil }, + { klass: BulkImports::Groups::Transformers::SubgroupToEntityTransformer, options: nil } ) end diff --git a/spec/lib/bulk_imports/importers/group_importer_spec.rb b/spec/lib/bulk_imports/importers/group_importer_spec.rb index 95ac5925c97..95dca7fc486 100644 --- a/spec/lib/bulk_imports/importers/group_importer_spec.rb +++ b/spec/lib/bulk_imports/importers/group_importer_spec.rb @@ -18,12 +18,12 @@ RSpec.describe BulkImports::Importers::GroupImporter do subject { described_class.new(bulk_import_entity) } before do + allow(Gitlab).to receive(:ee?).and_return(false) allow(BulkImports::Pipeline::Context).to receive(:new).and_return(context) - stub_http_requests end describe '#execute' do - it "starts the entity and run its pipelines" do + it 'starts the entity and run its pipelines' do expect(bulk_import_entity).to receive(:start).and_call_original expect_to_run_pipeline BulkImports::Groups::Pipelines::GroupPipeline, context: context expect_to_run_pipeline BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline, context: context @@ -32,6 +32,18 @@ RSpec.describe BulkImports::Importers::GroupImporter do expect(bulk_import_entity.reload).to be_finished end + + context 'when failed' do + let(:bulk_import_entity) { create(:bulk_import_entity, :failed, bulk_import: bulk_import) } + + it 'does not transition entity to finished state' do + allow(bulk_import_entity).to receive(:start!) + + subject.execute + + expect(bulk_import_entity.reload).to be_failed + end + end end def expect_to_run_pipeline(klass, context:) @@ -39,18 +51,4 @@ RSpec.describe BulkImports::Importers::GroupImporter do expect(pipeline).to receive(:run).with(context) end end - - def stub_http_requests - double_response = double( - code: 200, - success?: true, - parsed_response: {}, - headers: {} - ) - - allow_next_instance_of(BulkImports::Clients::Http) do |client| - allow(client).to receive(:get).and_return(double_response) - allow(client).to receive(:post).and_return(double_response) - end - end end diff --git a/spec/lib/bulk_imports/pipeline/runner_spec.rb b/spec/lib/bulk_imports/pipeline/runner_spec.rb index 8c882c799ec..60833e83dcc 100644 --- a/spec/lib/bulk_imports/pipeline/runner_spec.rb +++ b/spec/lib/bulk_imports/pipeline/runner_spec.rb @@ -3,26 +3,32 @@ require 'spec_helper' RSpec.describe BulkImports::Pipeline::Runner do - describe 'pipeline runner' do - before do - extractor = Class.new do - def initialize(options = {}); end + let(:extractor) do + Class.new do + def initialize(options = {}); end - def extract(context); end - end + def extract(context); end + end + end - transformer = Class.new do - def initialize(options = {}); end + let(:transformer) do + Class.new do + def initialize(options = {}); end - def transform(context, entry); end - end + def transform(context); end + end + end - loader = Class.new do - def initialize(options = {}); end + let(:loader) do + Class.new do + def initialize(options = {}); end - def load(context, entry); end - end + def load(context); end + end + end + describe 'pipeline runner' do + before do stub_const('BulkImports::Extractor', extractor) stub_const('BulkImports::Transformer', transformer) stub_const('BulkImports::Loader', loader) @@ -38,37 +44,126 @@ RSpec.describe BulkImports::Pipeline::Runner do stub_const('BulkImports::MyPipeline', pipeline) end - it 'runs pipeline extractor, transformer, loader' do - context = instance_double( - BulkImports::Pipeline::Context, - entity: instance_double(BulkImports::Entity, id: 1, source_type: 'group') - ) - entries = [{ foo: :bar }] - - expect_next_instance_of(BulkImports::Extractor) do |extractor| - expect(extractor).to receive(:extract).with(context).and_return(entries) + context 'when entity is not marked as failed' do + let(:context) do + instance_double( + BulkImports::Pipeline::Context, + entity: instance_double(BulkImports::Entity, id: 1, source_type: 'group', failed?: false) + ) end - expect_next_instance_of(BulkImports::Transformer) do |transformer| - expect(transformer).to receive(:transform).with(context, entries.first).and_return(entries.first) + it 'runs pipeline extractor, transformer, loader' do + entries = [{ foo: :bar }] + + expect_next_instance_of(BulkImports::Extractor) do |extractor| + expect(extractor).to receive(:extract).with(context).and_return(entries) + end + + expect_next_instance_of(BulkImports::Transformer) do |transformer| + expect(transformer).to receive(:transform).with(context, entries.first).and_return(entries.first) + end + + expect_next_instance_of(BulkImports::Loader) do |loader| + expect(loader).to receive(:load).with(context, entries.first) + end + + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger).to receive(:info) + .with( + message: 'Pipeline started', + pipeline_class: 'BulkImports::MyPipeline', + bulk_import_entity_id: 1, + bulk_import_entity_type: 'group' + ) + expect(logger).to receive(:info) + .with(bulk_import_entity_id: 1, bulk_import_entity_type: 'group', extractor: 'BulkImports::Extractor') + expect(logger).to receive(:info) + .with(bulk_import_entity_id: 1, bulk_import_entity_type: 'group', transformer: 'BulkImports::Transformer') + expect(logger).to receive(:info) + .with(bulk_import_entity_id: 1, bulk_import_entity_type: 'group', loader: 'BulkImports::Loader') + end + + BulkImports::MyPipeline.new.run(context) end - expect_next_instance_of(BulkImports::Loader) do |loader| - expect(loader).to receive(:load).with(context, entries.first) + context 'when exception is raised' do + let(:entity) { create(:bulk_import_entity, :created) } + let(:context) { BulkImports::Pipeline::Context.new(entity: entity) } + + before do + allow_next_instance_of(BulkImports::Extractor) do |extractor| + allow(extractor).to receive(:extract).with(context).and_raise(StandardError, 'Error!') + end + end + + it 'logs import failure' do + BulkImports::MyPipeline.new.run(context) + + failure = entity.failures.first + + expect(failure).to be_present + expect(failure.pipeline_class).to eq('BulkImports::MyPipeline') + expect(failure.exception_class).to eq('StandardError') + expect(failure.exception_message).to eq('Error!') + end + + context 'when pipeline is marked to abort on failure' do + before do + BulkImports::MyPipeline.abort_on_failure! + end + + it 'marks entity as failed' do + BulkImports::MyPipeline.new.run(context) + + expect(entity.failed?).to eq(true) + end + + it 'logs warn message' do + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger).to receive(:warn) + .with( + message: 'Pipeline failed', + pipeline_class: 'BulkImports::MyPipeline', + bulk_import_entity_id: entity.id, + bulk_import_entity_type: entity.source_type + ) + end + + BulkImports::MyPipeline.new.run(context) + end + end + + context 'when pipeline is not marked to abort on failure' do + it 'marks entity as failed' do + BulkImports::MyPipeline.new.run(context) + + expect(entity.failed?).to eq(false) + end + end end + end - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger).to receive(:info) - .with(message: "Pipeline started", pipeline: 'BulkImports::MyPipeline', entity: 1, entity_type: 'group') - expect(logger).to receive(:info) - .with(entity: 1, entity_type: 'group', extractor: 'BulkImports::Extractor') - expect(logger).to receive(:info) - .with(entity: 1, entity_type: 'group', transformer: 'BulkImports::Transformer') - expect(logger).to receive(:info) - .with(entity: 1, entity_type: 'group', loader: 'BulkImports::Loader') + context 'when entity is marked as failed' do + let(:context) do + instance_double( + BulkImports::Pipeline::Context, + entity: instance_double(BulkImports::Entity, id: 1, source_type: 'group', failed?: true) + ) end - BulkImports::MyPipeline.new.run(context) + it 'logs and returns without execution' do + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger).to receive(:info) + .with( + message: 'Skipping due to failed pipeline status', + pipeline_class: 'BulkImports::MyPipeline', + bulk_import_entity_id: 1, + bulk_import_entity_type: 'group' + ) + end + + BulkImports::MyPipeline.new.run(context) + end end end end diff --git a/spec/lib/bulk_imports/pipeline/attributes_spec.rb b/spec/lib/bulk_imports/pipeline_spec.rb index 54c5dbd4cae..94052be7df2 100644 --- a/spec/lib/bulk_imports/pipeline/attributes_spec.rb +++ b/spec/lib/bulk_imports/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Pipeline::Attributes do +RSpec.describe BulkImports::Pipeline do describe 'pipeline attributes' do before do stub_const('BulkImports::Extractor', Class.new) @@ -10,7 +10,9 @@ RSpec.describe BulkImports::Pipeline::Attributes do stub_const('BulkImports::Loader', Class.new) klass = Class.new do - include BulkImports::Pipeline::Attributes + include BulkImports::Pipeline + + abort_on_failure! extractor BulkImports::Extractor, { foo: :bar } transformer BulkImports::Transformer, { foo: :bar } @@ -25,6 +27,7 @@ RSpec.describe BulkImports::Pipeline::Attributes do expect(BulkImports::MyPipeline.extractors).to contain_exactly({ klass: BulkImports::Extractor, options: { foo: :bar } }) expect(BulkImports::MyPipeline.transformers).to contain_exactly({ klass: BulkImports::Transformer, options: { foo: :bar } }) expect(BulkImports::MyPipeline.loaders).to contain_exactly({ klass: BulkImports::Loader, options: { foo: :bar } }) + expect(BulkImports::MyPipeline.abort_on_failure?).to eq(true) end end @@ -36,6 +39,7 @@ RSpec.describe BulkImports::Pipeline::Attributes do BulkImports::MyPipeline.extractor(klass, options) BulkImports::MyPipeline.transformer(klass, options) BulkImports::MyPipeline.loader(klass, options) + BulkImports::MyPipeline.abort_on_failure! expect(BulkImports::MyPipeline.extractors) .to contain_exactly( @@ -51,6 +55,8 @@ RSpec.describe BulkImports::Pipeline::Attributes do .to contain_exactly( { klass: BulkImports::Loader, options: { foo: :bar } }, { klass: klass, options: options }) + + expect(BulkImports::MyPipeline.abort_on_failure?).to eq(true) end end end |