Merge branch 'kassio/bulkimports-simplify-context' into 'master'

Simplify BulkImports::Pipeline::Context

See merge request gitlab-org/gitlab!52833

ee/spec/lib/ee/bulk_imports/groups/loaders/epics_loader_spec.rb

@@ -7,12 +7,7 @@ RSpec.describe EE::BulkImports::Groups::Loaders::EpicsLoader do
     let(:user) { create(:user) }
     let(:group) { create(:group) }
     let(:entity) { create(:bulk_import_entity, group: group) }
-    let(:context) do
-      BulkImports::Pipeline::Context.new(
-        entity: entity,
-        current_user: user
-      )
-    end
+    let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
     let(:data) do
       {

ee/spec/lib/ee/bulk_imports/groups/pipelines/epics_pipeline_spec.rb

@@ -16,12 +16,7 @@ RSpec.describe EE::BulkImports::Groups::Pipelines::EpicsPipeline do
     )
   end
 
-  let(:context) do
-    BulkImports::Pipeline::Context.new(
-      current_user: user,
-      entity: entity
-    )
-  end
+  let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
   describe '#run' do
     it 'imports group epics into destination group' do

ee/spec/lib/ee/bulk_imports/importers/group_importer_spec.rb

@@ -4,16 +4,10 @@ require 'spec_helper'
 
 RSpec.describe BulkImports::Importers::GroupImporter do
   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_configuration) { create(:bulk_import_configuration, bulk_import: bulk_import) }
-  let(:context) do
-    BulkImports::Pipeline::Context.new(
-      current_user: user,
-      entity: bulk_import_entity,
-      configuration: bulk_import_configuration
-    )
-  end
+  let(:context) { BulkImports::Pipeline::Context.new(bulk_import_entity) }
 
   subject { described_class.new(bulk_import_entity) }
 

lib/bulk_imports/common/loaders/entity_loader.rb

@@ -7,7 +7,7 @@ module BulkImports
         def initialize(*args); end
 
         def load(context, entity)
-          context.entity.bulk_import.entities.create!(entity)
+          context.bulk_import.entities.create!(entity)
         end
       end
     end

lib/bulk_imports/groups/extractors/subgroups_extractor.rb

@@ -9,7 +9,7 @@ module BulkImports
         def extract(context)
           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")
             .flat_map(&:itself)
 

lib/bulk_imports/groups/loaders/labels_loader.rb

@@ -7,7 +7,7 @@ module BulkImports
         def initialize(*); end
 
         def load(context, data)
-          Labels::CreateService.new(data).execute(group: context.entity.group)
+          Labels::CreateService.new(data).execute(group: context.group)
         end
       end
     end

lib/bulk_imports/importers/group_importer.rb

@@ -8,14 +8,7 @@ module BulkImports
       end
 
       def execute
-        bulk_import = entity.bulk_import
-        configuration = bulk_import.configuration
-
-        context = BulkImports::Pipeline::Context.new(
-          current_user: bulk_import.user,
-          entity: entity,
-          configuration: configuration
-        )
+        context = BulkImports::Pipeline::Context.new(entity)
 
         pipelines.each { |pipeline| pipeline.new.run(context) }
 

lib/bulk_imports/pipeline/context.rb

@@ -3,30 +3,23 @@
 module BulkImports
   module Pipeline
     class Context
-      include Gitlab::Utils::LazyAttributes
+      attr_reader :entity, :bulk_import
 
-      Attribute = Struct.new(:name, :type)
-
-      PIPELINE_ATTRIBUTES = [
-        Attribute.new(:current_user, User),
-        Attribute.new(:entity, ::BulkImports::Entity),
-        Attribute.new(:configuration, ::BulkImports::Configuration)
-      ].freeze
-
-      def initialize(args)
-        assign_attributes(args)
+      def initialize(entity)
+        @entity = entity
+        @bulk_import = entity.bulk_import
       end
 
-      private
+      def group
+        entity.group
+      end
 
-      PIPELINE_ATTRIBUTES.each do |attr|
-        lazy_attr_reader attr.name, type: attr.type
+      def current_user
+        bulk_import.user
       end
 
-      def assign_attributes(values)
-        values.slice(*PIPELINE_ATTRIBUTES.map(&:name)).each do |name, value|
-          instance_variable_set("@#{name}", value)
-        end
+      def configuration
+        bulk_import.configuration
       end
     end
   end

spec/lib/bulk_imports/common/loaders/entity_loader_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe BulkImports::Common::Loaders::EntityLoader do
     it "creates entities for the given data" do
       group = create(:group, path: "imported-group")
       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 = {
         source_type: :group_entity,

spec/lib/bulk_imports/groups/extractors/subgroups_extractor_spec.rb

@@ -5,16 +5,11 @@ require 'spec_helper'
 RSpec.describe BulkImports::Groups::Extractors::SubgroupsExtractor do
   describe '#extract' do
     it 'returns ExtractedData response' do
-      user = create(:user)
       bulk_import = create(:bulk_import)
+      create(:bulk_import_configuration, 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' }]
-      context = BulkImports::Pipeline::Context.new(
-        current_user: user,
-        entity: entity,
-        configuration: configuration
-      )
+      context = BulkImports::Pipeline::Context.new(entity)
 
       allow_next_instance_of(BulkImports::Clients::Http) do |client|
         allow(client).to receive(:each_page).and_return(response)

spec/lib/bulk_imports/groups/loaders/group_loader_spec.rb

@@ -7,21 +7,20 @@ RSpec.describe BulkImports::Groups::Loaders::GroupLoader do
     let(:user) { create(:user) }
     let(:data) { { foo: :bar } }
     let(:service_double) { instance_double(::Groups::CreateService) }
-    let(:entity) { create(:bulk_import_entity) }
-    let(:context) do
-      instance_double(
-        BulkImports::Pipeline::Context,
-        entity: entity,
-        current_user: user
-      )
-    end
+    let(:bulk_import) { create(:bulk_import, user: user) }
+    let(:entity) { create(:bulk_import_entity, bulk_import: bulk_import) }
+    let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
     subject { described_class.new }
 
     context 'when user can create 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
-          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(entity).to receive(:update!)
 

spec/lib/bulk_imports/groups/loaders/labels_loader_spec.rb

@@ -7,12 +7,7 @@ RSpec.describe BulkImports::Groups::Loaders::LabelsLoader do
     let(:user) { create(:user) }
     let(:group) { create(:group) }
     let(:entity) { create(:bulk_import_entity, group: group) }
-    let(:context) do
-      BulkImports::Pipeline::Context.new(
-        entity: entity,
-        current_user: user
-      )
-    end
+    let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
     let(:data) do
       {

spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb

@@ -6,21 +6,18 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do
   describe '#run' do
     let(:user) { create(:user) }
     let(:parent) { create(:group) }
+    let(:bulk_import) { create(:bulk_import, user: user) }
     let(:entity) do
       create(
         :bulk_import_entity,
+        bulk_import: bulk_import,
         source_full_path: 'source/full/path',
         destination_name: 'My Destination Group',
         destination_namespace: parent.full_path
       )
     end
 
-    let(:context) do
-      BulkImports::Pipeline::Context.new(
-        current_user: user,
-        entity: entity
-      )
-    end
+    let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
     let(:group_data) do
       {
@@ -36,8 +33,6 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do
       }
     end
 
-    subject { described_class.new }
-
     before do
       allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor|
         allow(extractor).to receive(:extract).and_return([group_data])

spec/lib/bulk_imports/groups/pipelines/labels_pipeline_spec.rb

@@ -16,12 +16,7 @@ RSpec.describe BulkImports::Groups::Pipelines::LabelsPipeline do
     )
   end
 
-  let(:context) do
-    BulkImports::Pipeline::Context.new(
-      current_user: user,
-      entity: entity
-    )
-  end
+  let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
   def extractor_data(title:, has_next_page:, cursor: nil)
     data = [

spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb

@@ -14,13 +14,7 @@ RSpec.describe BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline do
       )
     end
 
-    let(:context) do
-      instance_double(
-        BulkImports::Pipeline::Context,
-        current_user: user,
-        entity: parent_entity
-      )
-    end
+    let(:context) { BulkImports::Pipeline::Context.new(parent_entity) }
 
     let(:subgroup_data) do
       [

spec/lib/bulk_imports/groups/transformers/group_attributes_transformer_spec.rb

@@ -7,22 +7,18 @@ RSpec.describe BulkImports::Groups::Transformers::GroupAttributesTransformer do
     let(:user) { create(:user) }
     let(:parent) { create(:group) }
     let(:group) { create(:group, name: 'My Source Group', parent: parent) }
+    let(:bulk_import) { create(:bulk_import, user: user) }
     let(:entity) do
-      instance_double(
-        BulkImports::Entity,
+      create(
+        :bulk_import_entity,
+        bulk_import: bulk_import,
         source_full_path: 'source/full/path',
         destination_name: group.name,
         destination_namespace: parent.full_path
       )
     end
 
-    let(:context) do
-      instance_double(
-        BulkImports::Pipeline::Context,
-        current_user: user,
-        entity: entity
-      )
-    end
+    let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
     let(:data) do
       {
@@ -85,16 +81,16 @@ RSpec.describe BulkImports::Groups::Transformers::GroupAttributesTransformer do
       end
 
       context 'when destination namespace is user namespace' do
-        let(:entity) do
-          instance_double(
-            BulkImports::Entity,
+        it 'does not set parent id' do
+          entity = create(
+            :bulk_import_entity,
+            bulk_import: bulk_import,
             source_full_path: 'source/full/path',
             destination_name: group.name,
             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)
 
           expect(transformed_data).not_to have_key('parent_id')

spec/lib/bulk_imports/importers/group_importer_spec.rb

@@ -7,20 +7,14 @@ RSpec.describe BulkImports::Importers::GroupImporter do
   let(:bulk_import) { create(: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(:context) do
-    BulkImports::Pipeline::Context.new(
-      current_user: user,
-      entity: bulk_import_entity,
-      configuration: bulk_import_configuration
-    )
-  end
-
-  subject { described_class.new(bulk_import_entity) }
+  let(:context) { BulkImports::Pipeline::Context.new(bulk_import_entity) }
 
   before do
     allow(BulkImports::Pipeline::Context).to receive(:new).and_return(context)
   end
 
+  subject { described_class.new(bulk_import_entity) }
+
   describe '#execute' do
     it 'starts the entity and run its pipelines' do
       expect_to_run_pipeline BulkImports::Groups::Pipelines::GroupPipeline, context: context

spec/lib/bulk_imports/pipeline/context_spec.rb

@@ -3,25 +3,29 @@
 require 'spec_helper'
 
 RSpec.describe BulkImports::Pipeline::Context do
-  describe '#initialize' do
-    it 'initializes with permitted attributes' do
-      args = {
-        current_user: create(:user),
-        entity: create(:bulk_import_entity),
-        configuration: create(:bulk_import_configuration)
-      }
+  let(:group) { instance_double(Group) }
+  let(:user) { instance_double(User) }
+  let(:bulk_import) { instance_double(BulkImport, user: user, configuration: :config) }
 
-      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|
-        expect(context.public_send(k)).to eq(v)
-      end
-    end
+  describe '#group' do
+    it { expect(subject.group).to eq(group) }
+  end
+
+  describe '#current_user' do
+    it { expect(subject.current_user).to eq(user) }
+  end
 
-    context 'when invalid argument is passed' do
-      it 'raises NoMethodError' do
-        expect { described_class.new(test: 'test').test }.to raise_exception(NoMethodError)
-      end
-    end
+  describe '#current_user' do
+    it { expect(subject.configuration).to eq(bulk_import.configuration) }
   end
 end

spec/lib/bulk_imports/pipeline/runner_spec.rb

@@ -45,12 +45,8 @@ RSpec.describe BulkImports::Pipeline::Runner do
     end
 
     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
+      let(:entity) { create(:bulk_import_entity) }
+      let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
       it 'runs pipeline extractor, transformer, loader' do
         extracted_data = BulkImports::Pipeline::ExtractedData.new(data: { foo: :bar })
@@ -80,15 +76,27 @@ RSpec.describe BulkImports::Pipeline::Runner do
             .with(
               message: 'Pipeline started',
               pipeline_class: 'BulkImports::MyPipeline',
-              bulk_import_entity_id: 1,
-              bulk_import_entity_type: 'group'
+              bulk_import_entity_id: entity.id,
+              bulk_import_entity_type: 'group_entity'
             )
           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)
-            .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)
-            .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
 
         BulkImports::MyPipeline.new.run(context)
@@ -96,7 +104,7 @@ RSpec.describe BulkImports::Pipeline::Runner do
 
       context 'when exception is raised' do
         let(:entity) { create(:bulk_import_entity, :created) }
-        let(:context) { BulkImports::Pipeline::Context.new(entity: entity) }
+        let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
         before do
           allow_next_instance_of(BulkImports::Extractor) do |extractor|
@@ -153,21 +161,19 @@ RSpec.describe BulkImports::Pipeline::Runner do
     end
 
     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
+      let(:entity) { create(:bulk_import_entity) }
+      let(:context) { BulkImports::Pipeline::Context.new(entity) }
 
       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(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'
+              bulk_import_entity_id: entity.id,
+              bulk_import_entity_type: 'group_entity'
             )
         end