Commit 4d242d85 authored by Kassio Borges's avatar Kassio Borges

Simplify BulkImports::Pipeline::Context

As the Gitlab Migration Grows (BulkImports) more and more developers
have to access the Pipeline context data, like the entity group. To make
this easier, the Context now has syntax-sugar methods for the most
accessed data.
parent 620bf1c9
...@@ -7,12 +7,7 @@ RSpec.describe EE::BulkImports::Groups::Loaders::EpicsLoader do ...@@ -7,12 +7,7 @@ RSpec.describe EE::BulkImports::Groups::Loaders::EpicsLoader do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:group) { create(:group) } let(:group) { create(:group) }
let(:entity) { create(:bulk_import_entity, group: group) } let(:entity) { create(:bulk_import_entity, group: group) }
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context.new(
entity: entity,
current_user: user
)
end
let(:data) do let(:data) do
{ {
......
...@@ -16,12 +16,7 @@ RSpec.describe EE::BulkImports::Groups::Pipelines::EpicsPipeline do ...@@ -16,12 +16,7 @@ RSpec.describe EE::BulkImports::Groups::Pipelines::EpicsPipeline do
) )
end end
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context.new(
current_user: user,
entity: entity
)
end
describe '#run' do describe '#run' do
it 'imports group epics into destination group' do it 'imports group epics into destination group' do
......
...@@ -4,16 +4,10 @@ require 'spec_helper' ...@@ -4,16 +4,10 @@ require 'spec_helper'
RSpec.describe BulkImports::Importers::GroupImporter do RSpec.describe BulkImports::Importers::GroupImporter do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:bulk_import) { create(:bulk_import) } let(:bulk_import) { create(:bulk_import, user: user) }
let(:bulk_import_entity) { create(:bulk_import_entity, :started, bulk_import: bulk_import) } let(:bulk_import_entity) { create(:bulk_import_entity, :started, bulk_import: bulk_import) }
let(:bulk_import_configuration) { create(:bulk_import_configuration, bulk_import: bulk_import) } let(:bulk_import_configuration) { create(:bulk_import_configuration, bulk_import: bulk_import) }
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(bulk_import_entity) }
BulkImports::Pipeline::Context.new(
current_user: user,
entity: bulk_import_entity,
configuration: bulk_import_configuration
)
end
subject { described_class.new(bulk_import_entity) } subject { described_class.new(bulk_import_entity) }
......
...@@ -7,7 +7,7 @@ module BulkImports ...@@ -7,7 +7,7 @@ module BulkImports
def initialize(*args); end def initialize(*args); end
def load(context, entity) def load(context, entity)
context.entity.bulk_import.entities.create!(entity) context.bulk_import.entities.create!(entity)
end end
end end
end end
......
...@@ -9,7 +9,7 @@ module BulkImports ...@@ -9,7 +9,7 @@ module BulkImports
def extract(context) def extract(context)
encoded_parent_path = ERB::Util.url_encode(context.entity.source_full_path) encoded_parent_path = ERB::Util.url_encode(context.entity.source_full_path)
response = http_client(context.entity.bulk_import.configuration) response = http_client(context.configuration)
.each_page(:get, "groups/#{encoded_parent_path}/subgroups") .each_page(:get, "groups/#{encoded_parent_path}/subgroups")
.flat_map(&:itself) .flat_map(&:itself)
......
...@@ -7,7 +7,7 @@ module BulkImports ...@@ -7,7 +7,7 @@ module BulkImports
def initialize(*); end def initialize(*); end
def load(context, data) def load(context, data)
Labels::CreateService.new(data).execute(group: context.entity.group) Labels::CreateService.new(data).execute(group: context.group)
end end
end end
end end
......
...@@ -8,14 +8,7 @@ module BulkImports ...@@ -8,14 +8,7 @@ module BulkImports
end end
def execute def execute
bulk_import = entity.bulk_import context = BulkImports::Pipeline::Context.new(entity)
configuration = bulk_import.configuration
context = BulkImports::Pipeline::Context.new(
current_user: bulk_import.user,
entity: entity,
configuration: configuration
)
pipelines.each { |pipeline| pipeline.new.run(context) } pipelines.each { |pipeline| pipeline.new.run(context) }
......
...@@ -3,30 +3,23 @@ ...@@ -3,30 +3,23 @@
module BulkImports module BulkImports
module Pipeline module Pipeline
class Context class Context
include Gitlab::Utils::LazyAttributes attr_reader :entity, :bulk_import
Attribute = Struct.new(:name, :type) def initialize(entity)
@entity = entity
PIPELINE_ATTRIBUTES = [ @bulk_import = entity.bulk_import
Attribute.new(:current_user, User),
Attribute.new(:entity, ::BulkImports::Entity),
Attribute.new(:configuration, ::BulkImports::Configuration)
].freeze
def initialize(args)
assign_attributes(args)
end end
private def group
entity.group
end
PIPELINE_ATTRIBUTES.each do |attr| def current_user
lazy_attr_reader attr.name, type: attr.type bulk_import.user
end end
def assign_attributes(values) def configuration
values.slice(*PIPELINE_ATTRIBUTES.map(&:name)).each do |name, value| bulk_import.configuration
instance_variable_set("@#{name}", value)
end
end end
end end
end end
......
...@@ -7,7 +7,7 @@ RSpec.describe BulkImports::Common::Loaders::EntityLoader do ...@@ -7,7 +7,7 @@ RSpec.describe BulkImports::Common::Loaders::EntityLoader do
it "creates entities for the given data" do it "creates entities for the given data" do
group = create(:group, path: "imported-group") group = create(:group, path: "imported-group")
parent_entity = create(:bulk_import_entity, group: group, bulk_import: create(:bulk_import)) parent_entity = create(:bulk_import_entity, group: group, bulk_import: create(:bulk_import))
context = instance_double(BulkImports::Pipeline::Context, entity: parent_entity) context = BulkImports::Pipeline::Context.new(parent_entity)
data = { data = {
source_type: :group_entity, source_type: :group_entity,
......
...@@ -5,16 +5,11 @@ require 'spec_helper' ...@@ -5,16 +5,11 @@ require 'spec_helper'
RSpec.describe BulkImports::Groups::Extractors::SubgroupsExtractor do RSpec.describe BulkImports::Groups::Extractors::SubgroupsExtractor do
describe '#extract' do describe '#extract' do
it 'returns ExtractedData response' do it 'returns ExtractedData response' do
user = create(:user)
bulk_import = create(:bulk_import) bulk_import = create(:bulk_import)
create(:bulk_import_configuration, bulk_import: bulk_import)
entity = create(:bulk_import_entity, bulk_import: bulk_import) entity = create(:bulk_import_entity, bulk_import: bulk_import)
configuration = create(:bulk_import_configuration, bulk_import: bulk_import)
response = [{ 'test' => 'group' }] response = [{ 'test' => 'group' }]
context = BulkImports::Pipeline::Context.new( context = BulkImports::Pipeline::Context.new(entity)
current_user: user,
entity: entity,
configuration: configuration
)
allow_next_instance_of(BulkImports::Clients::Http) do |client| allow_next_instance_of(BulkImports::Clients::Http) do |client|
allow(client).to receive(:each_page).and_return(response) allow(client).to receive(:each_page).and_return(response)
......
...@@ -7,21 +7,20 @@ RSpec.describe BulkImports::Groups::Loaders::GroupLoader do ...@@ -7,21 +7,20 @@ RSpec.describe BulkImports::Groups::Loaders::GroupLoader do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:data) { { foo: :bar } } let(:data) { { foo: :bar } }
let(:service_double) { instance_double(::Groups::CreateService) } let(:service_double) { instance_double(::Groups::CreateService) }
let(:entity) { create(:bulk_import_entity) } let(:bulk_import) { create(:bulk_import, user: user) }
let(:context) do let(:entity) { create(:bulk_import_entity, bulk_import: bulk_import) }
instance_double( let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context,
entity: entity,
current_user: user
)
end
subject { described_class.new } subject { described_class.new }
context 'when user can create group' do context 'when user can create group' do
shared_examples 'calls Group Create Service to create a new group' do shared_examples 'calls Group Create Service to create a new group' do
it 'calls Group Create Service to create a new group' do it 'calls Group Create Service to create a new group' do
expect(::Groups::CreateService).to receive(:new).with(context.current_user, data).and_return(service_double) expect(::Groups::CreateService)
.to receive(:new)
.with(context.current_user, data)
.and_return(service_double)
expect(service_double).to receive(:execute) expect(service_double).to receive(:execute)
expect(entity).to receive(:update!) expect(entity).to receive(:update!)
......
...@@ -7,12 +7,7 @@ RSpec.describe BulkImports::Groups::Loaders::LabelsLoader do ...@@ -7,12 +7,7 @@ RSpec.describe BulkImports::Groups::Loaders::LabelsLoader do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:group) { create(:group) } let(:group) { create(:group) }
let(:entity) { create(:bulk_import_entity, group: group) } let(:entity) { create(:bulk_import_entity, group: group) }
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context.new(
entity: entity,
current_user: user
)
end
let(:data) do let(:data) do
{ {
......
...@@ -6,21 +6,18 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do ...@@ -6,21 +6,18 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do
describe '#run' do describe '#run' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:parent) { create(:group) } let(:parent) { create(:group) }
let(:bulk_import) { create(:bulk_import, user: user) }
let(:entity) do let(:entity) do
create( create(
:bulk_import_entity, :bulk_import_entity,
bulk_import: bulk_import,
source_full_path: 'source/full/path', source_full_path: 'source/full/path',
destination_name: 'My Destination Group', destination_name: 'My Destination Group',
destination_namespace: parent.full_path destination_namespace: parent.full_path
) )
end end
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context.new(
current_user: user,
entity: entity
)
end
let(:group_data) do let(:group_data) do
{ {
...@@ -36,8 +33,6 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do ...@@ -36,8 +33,6 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do
} }
end end
subject { described_class.new }
before do before do
allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor| allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor|
allow(extractor).to receive(:extract).and_return([group_data]) allow(extractor).to receive(:extract).and_return([group_data])
......
...@@ -16,12 +16,7 @@ RSpec.describe BulkImports::Groups::Pipelines::LabelsPipeline do ...@@ -16,12 +16,7 @@ RSpec.describe BulkImports::Groups::Pipelines::LabelsPipeline do
) )
end end
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context.new(
current_user: user,
entity: entity
)
end
def extractor_data(title:, has_next_page:, cursor: nil) def extractor_data(title:, has_next_page:, cursor: nil)
data = [ data = [
......
...@@ -14,13 +14,7 @@ RSpec.describe BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline do ...@@ -14,13 +14,7 @@ RSpec.describe BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline do
) )
end end
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(parent_entity) }
instance_double(
BulkImports::Pipeline::Context,
current_user: user,
entity: parent_entity
)
end
let(:subgroup_data) do let(:subgroup_data) do
[ [
......
...@@ -7,22 +7,18 @@ RSpec.describe BulkImports::Groups::Transformers::GroupAttributesTransformer do ...@@ -7,22 +7,18 @@ RSpec.describe BulkImports::Groups::Transformers::GroupAttributesTransformer do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:parent) { create(:group) } let(:parent) { create(:group) }
let(:group) { create(:group, name: 'My Source Group', parent: parent) } let(:group) { create(:group, name: 'My Source Group', parent: parent) }
let(:bulk_import) { create(:bulk_import, user: user) }
let(:entity) do let(:entity) do
instance_double( create(
BulkImports::Entity, :bulk_import_entity,
bulk_import: bulk_import,
source_full_path: 'source/full/path', source_full_path: 'source/full/path',
destination_name: group.name, destination_name: group.name,
destination_namespace: parent.full_path destination_namespace: parent.full_path
) )
end end
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(entity) }
instance_double(
BulkImports::Pipeline::Context,
current_user: user,
entity: entity
)
end
let(:data) do let(:data) do
{ {
...@@ -85,16 +81,16 @@ RSpec.describe BulkImports::Groups::Transformers::GroupAttributesTransformer do ...@@ -85,16 +81,16 @@ RSpec.describe BulkImports::Groups::Transformers::GroupAttributesTransformer do
end end
context 'when destination namespace is user namespace' do context 'when destination namespace is user namespace' do
let(:entity) do it 'does not set parent id' do
instance_double( entity = create(
BulkImports::Entity, :bulk_import_entity,
bulk_import: bulk_import,
source_full_path: 'source/full/path', source_full_path: 'source/full/path',
destination_name: group.name, destination_name: group.name,
destination_namespace: user.namespace.full_path destination_namespace: user.namespace.full_path
) )
end context = BulkImports::Pipeline::Context.new(entity)
it 'does not set parent id' do
transformed_data = subject.transform(context, data) transformed_data = subject.transform(context, data)
expect(transformed_data).not_to have_key('parent_id') expect(transformed_data).not_to have_key('parent_id')
......
...@@ -7,20 +7,14 @@ RSpec.describe BulkImports::Importers::GroupImporter do ...@@ -7,20 +7,14 @@ RSpec.describe BulkImports::Importers::GroupImporter do
let(:bulk_import) { create(:bulk_import) } let(:bulk_import) { create(:bulk_import) }
let(:bulk_import_entity) { create(:bulk_import_entity, :started, bulk_import: bulk_import) } let(:bulk_import_entity) { create(:bulk_import_entity, :started, bulk_import: bulk_import) }
let(:bulk_import_configuration) { create(:bulk_import_configuration, bulk_import: bulk_import) } let(:bulk_import_configuration) { create(:bulk_import_configuration, bulk_import: bulk_import) }
let(:context) do let(:context) { BulkImports::Pipeline::Context.new(bulk_import_entity) }
BulkImports::Pipeline::Context.new(
current_user: user,
entity: bulk_import_entity,
configuration: bulk_import_configuration
)
end
subject { described_class.new(bulk_import_entity) }
before do before do
allow(BulkImports::Pipeline::Context).to receive(:new).and_return(context) allow(BulkImports::Pipeline::Context).to receive(:new).and_return(context)
end end
subject { described_class.new(bulk_import_entity) }
describe '#execute' do describe '#execute' do
it 'starts the entity and run its pipelines' do it 'starts the entity and run its pipelines' do
expect_to_run_pipeline BulkImports::Groups::Pipelines::GroupPipeline, context: context expect_to_run_pipeline BulkImports::Groups::Pipelines::GroupPipeline, context: context
......
...@@ -3,25 +3,29 @@ ...@@ -3,25 +3,29 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe BulkImports::Pipeline::Context do RSpec.describe BulkImports::Pipeline::Context do
describe '#initialize' do let(:group) { instance_double(Group) }
it 'initializes with permitted attributes' do let(:user) { instance_double(User) }
args = { let(:bulk_import) { instance_double(BulkImport, user: user, configuration: :config) }
current_user: create(:user),
entity: create(:bulk_import_entity),
configuration: create(:bulk_import_configuration)
}
context = described_class.new(args) let(:entity) do
instance_double(
BulkImports::Entity,
bulk_import: bulk_import,
group: group
)
end
subject { described_class.new(entity) }
args.each do |k, v| describe '#group' do
expect(context.public_send(k)).to eq(v) it { expect(subject.group).to eq(group) }
end end
end
describe '#current_user' do
it { expect(subject.current_user).to eq(user) }
end
context 'when invalid argument is passed' do describe '#current_user' do
it 'raises NoMethodError' do it { expect(subject.configuration).to eq(bulk_import.configuration) }
expect { described_class.new(test: 'test').test }.to raise_exception(NoMethodError)
end
end
end end
end end
...@@ -45,12 +45,8 @@ RSpec.describe BulkImports::Pipeline::Runner do ...@@ -45,12 +45,8 @@ RSpec.describe BulkImports::Pipeline::Runner do
end end
context 'when entity is not marked as failed' do context 'when entity is not marked as failed' do
let(:context) do let(:entity) { create(:bulk_import_entity) }
instance_double( let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context,
entity: instance_double(BulkImports::Entity, id: 1, source_type: 'group', failed?: false)
)
end
it 'runs pipeline extractor, transformer, loader' do it 'runs pipeline extractor, transformer, loader' do
extracted_data = BulkImports::Pipeline::ExtractedData.new(data: { foo: :bar }) extracted_data = BulkImports::Pipeline::ExtractedData.new(data: { foo: :bar })
...@@ -80,15 +76,27 @@ RSpec.describe BulkImports::Pipeline::Runner do ...@@ -80,15 +76,27 @@ RSpec.describe BulkImports::Pipeline::Runner do
.with( .with(
message: 'Pipeline started', message: 'Pipeline started',
pipeline_class: 'BulkImports::MyPipeline', pipeline_class: 'BulkImports::MyPipeline',
bulk_import_entity_id: 1, bulk_import_entity_id: entity.id,
bulk_import_entity_type: 'group' bulk_import_entity_type: 'group_entity'
) )
expect(logger).to receive(:info) expect(logger).to receive(:info)
.with(bulk_import_entity_id: 1, bulk_import_entity_type: 'group', extractor: 'BulkImports::Extractor') .with(
bulk_import_entity_id: entity.id,
bulk_import_entity_type: 'group_entity',
extractor: 'BulkImports::Extractor'
)
expect(logger).to receive(:info) expect(logger).to receive(:info)
.with(bulk_import_entity_id: 1, bulk_import_entity_type: 'group', transformer: 'BulkImports::Transformer') .with(
bulk_import_entity_id: entity.id,
bulk_import_entity_type: 'group_entity',
transformer: 'BulkImports::Transformer'
)
expect(logger).to receive(:info) expect(logger).to receive(:info)
.with(bulk_import_entity_id: 1, bulk_import_entity_type: 'group', loader: 'BulkImports::Loader') .with(
bulk_import_entity_id: entity.id,
bulk_import_entity_type: 'group_entity',
loader: 'BulkImports::Loader'
)
end end
BulkImports::MyPipeline.new.run(context) BulkImports::MyPipeline.new.run(context)
...@@ -96,7 +104,7 @@ RSpec.describe BulkImports::Pipeline::Runner do ...@@ -96,7 +104,7 @@ RSpec.describe BulkImports::Pipeline::Runner do
context 'when exception is raised' do context 'when exception is raised' do
let(:entity) { create(:bulk_import_entity, :created) } let(:entity) { create(:bulk_import_entity, :created) }
let(:context) { BulkImports::Pipeline::Context.new(entity: entity) } let(:context) { BulkImports::Pipeline::Context.new(entity) }
before do before do
allow_next_instance_of(BulkImports::Extractor) do |extractor| allow_next_instance_of(BulkImports::Extractor) do |extractor|
...@@ -153,21 +161,19 @@ RSpec.describe BulkImports::Pipeline::Runner do ...@@ -153,21 +161,19 @@ RSpec.describe BulkImports::Pipeline::Runner do
end end
context 'when entity is marked as failed' do context 'when entity is marked as failed' do
let(:context) do let(:entity) { create(:bulk_import_entity) }
instance_double( let(:context) { BulkImports::Pipeline::Context.new(entity) }
BulkImports::Pipeline::Context,
entity: instance_double(BulkImports::Entity, id: 1, source_type: 'group', failed?: true)
)
end
it 'logs and returns without execution' do it 'logs and returns without execution' do
allow(entity).to receive(:failed?).and_return(true)
expect_next_instance_of(Gitlab::Import::Logger) do |logger| expect_next_instance_of(Gitlab::Import::Logger) do |logger|
expect(logger).to receive(:info) expect(logger).to receive(:info)
.with( .with(
message: 'Skipping due to failed pipeline status', message: 'Skipping due to failed pipeline status',
pipeline_class: 'BulkImports::MyPipeline', pipeline_class: 'BulkImports::MyPipeline',
bulk_import_entity_id: 1, bulk_import_entity_id: entity.id,
bulk_import_entity_type: 'group' bulk_import_entity_type: 'group_entity'
) )
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