Refactor importers to use Hyrax's actor stack

lib/importer/factory/collection_factory.rb

@@ -10,8 +10,13 @@ def find_or_create
         run(&:save!)
       end
 
-      def attach_files
-        # nop
+      def update
+        raise "Collection doesn't exist" unless object
+        object.attributes = update_attributes
+        run_callbacks(:save) do
+          object.save!
+        end
+        log_updated(object)
       end
     end
   end

lib/importer/factory/etd_factory.rb

@@ -4,7 +4,6 @@ class ETDFactory < ObjectFactory
       include WithAssociatedCollection
 
       self.klass = GenericWork
-      self.attach_files_service = Importer::AttachFiles
       # A way to identify objects that are not Hydra minted identifiers
       self.system_identifier_field = :identifier
 

lib/importer/factory/image_factory.rb

@@ -4,7 +4,6 @@ class ImageFactory < ObjectFactory
       include WithAssociatedCollection
 
       self.klass = GenericWork
-      self.attach_files_service = Importer::AttachFiles
       # A way to identify objects that are not Hydra minted identifiers
       self.system_identifier_field = :identifier
 

lib/importer/factory/object_factory.rb

@@ -4,9 +4,8 @@ module Factory
     class ObjectFactory
       extend ActiveModel::Callbacks
       define_model_callbacks :save, :create
-      after_save :attach_files
 
-      class_attribute :klass, :attach_files_service, :system_identifier_field
+      class_attribute :klass, :system_identifier_field
 
       attr_reader :attributes, :files_directory, :object, :files
 
@@ -39,9 +38,8 @@ def run
 
       def update
         raise "Object doesn't exist" unless object
-        object.attributes = update_attributes
         run_callbacks(:save) do
-          object.save!
+          work_actor.update(update_attributes)
         end
         log_updated(object)
       end
@@ -54,13 +52,6 @@ def update_attributes
         transform_attributes.except(:id)
       end
 
-      def attach_files
-        return unless files_directory.present? && files.present?
-
-        attach_files_service.run(object, files_directory, files)
-        object.save! # Save the association with the attached files.
-      end
-
       def find
         return find_by_id if attributes[:id]
         return search_by_identifier if attributes[system_identifier_field].present?
@@ -78,7 +69,7 @@ def search_by_identifier
         klass.where(query).first
       end
 
-      # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
+      # rubocop:disable Metrics/MethodLength
       def create
         attrs = create_attributes
 
@@ -93,7 +84,6 @@ def create
               @object.apply_depositor_metadata(User.batch_user)
               @object.save!
             else
-              work_actor = Hyrax::CurationConcern.actor(@object, Ability.new(User.batch_user))
               work_actor.create(attrs)
             end
           end
@@ -103,6 +93,10 @@ def create
       end
       # rubocop:enable Metrics/MethodLength, Metrics/AbcSize
 
+      def work_actor
+        Hyrax::CurationConcern.actor(@object, Ability.new(User.batch_user))
+      end
+
       def log_created(obj)
         Rails.logger.info(
           "Created #{klass.model_name.human} #{obj.id} (#{Array(attributes[system_identifier_field]).first})"
@@ -121,6 +115,15 @@ def log_updated(obj)
         # a way that is compatible with how the factory needs them.
         def transform_attributes
           StringLiteralProcessor.process(attributes.slice(*permitted_attributes))
+                                .merge(file_attributes)
+        end
+
+        def file_attributes
+          files_directory.present? && files.present? ? { files: file_paths } : {}
+        end
+
+        def file_paths
+          files.map { |file_name| File.join(files_directory, file_name) }
         end
 
         # Regardless of what the MODS Parser gives us, these are the properties

lib/importer/factory/with_associated_collection.rb

@@ -3,29 +3,16 @@ module Factory
     module WithAssociatedCollection
       extend ActiveSupport::Concern
 
-      included do
-        after_save :maybe_add_to_collection
-      end
-
+      # Strip out the :collection key, and add the member_of_collection_ids,
+      # which is used by Hyrax::Actors::AddAsMemberOfCollectionsActor
       def create_attributes
-        super.except(:collection)
+        super.except(:collection).merge(member_of_collection_ids: [collection.id])
       end
 
+      # Strip out the :collection key, and add the member_of_collection_ids,
+      # which is used by Hyrax::Actors::AddAsMemberOfCollectionsActor
       def update_attributes
-        super.except(:collection)
-      end
-
-      def maybe_add_to_collection
-        return unless attributes.key?(:collection)
-        add_to_collection(object, collection)
-
-        # Reindex the object with the collection label.
-        object.update_index
-      end
-
-      def add_to_collection(obj, collection)
-        collection.ordered_members << obj
-        collection.save!
+        super.except(:collection).merge(member_of_collection_ids: [collection.id])
       end
 
       private

spec/lib/importer/factory/etd_factory_spec.rb

@@ -2,7 +2,10 @@
 
 RSpec.describe Importer::Factory::ETDFactory, :clean do
   let(:factory) { described_class.new(attributes) }
-
+  let(:actor) { instance_double(Hyrax::Actors::ActorStack) }
+  before do
+    allow(Hyrax::CurationConcern).to receive(:actor).and_return(actor)
+  end
   let(:files) { [] }
   let(:attributes) do
     {
@@ -19,47 +22,37 @@
   context 'with files' do
     let(:factory) { described_class.new(attributes, 'tmp/files', files) }
     let(:files) { ['img.png'] }
-    let(:file) { double('the file') }
-    let!(:coll) { FactoryGirl.create(:collection) }
-    let(:fs_actor) { double }
-    before do
-      allow(File).to receive(:exist?).and_call_original # For byebug, sprockets, etc
-      allow(File).to receive(:exist?).with("tmp/files/img.png").and_return(true)
-      allow(File).to receive(:new).and_return(file)
-      allow(Hyrax::Actors::FileSetActor).to receive(:new)
-        .with(FileSet, User).and_return(fs_actor)
-    end
+    let(:coll) { create(:collection) }
+
     context "for a new image" do
-      it 'creates file sets with access controls' do
-        expect(fs_actor).to receive(:create_metadata).with(GenericWork)
-        expect(fs_actor).to receive(:create_content).with(file)
+      it 'calls the actor with the files' do
+        expect(actor).to receive(:create).with(
+          hash_including(member_of_collection_ids: [coll.id], files: ['tmp/files/img.png'])
+        )
         factory.run
       end
     end
 
     context "for an existing image without files" do
-      let!(:work) { FactoryGirl.create(:generic_work) }
+      let(:work) { create(:generic_work) }
       let(:factory) { described_class.new(attributes.merge(id: work.id), 'tmp/files', files) }
       it 'creates file sets' do
-        expect do
-          expect(fs_actor).to receive(:create_metadata).with(work)
-          expect(fs_actor).to receive(:create_content).with(file)
-          factory.run
-        end.not_to change { GenericWork.count }
+        expect(actor).to receive(:update).with(
+          hash_including(member_of_collection_ids: [coll.id], files: ['tmp/files/img.png'])
+        )
+        factory.run
       end
     end
   end
 
   context 'when a collection already exists' do
-    let!(:coll) { FactoryGirl.create(:collection) }
+    let(:coll) { create(:collection) }
 
     it 'does not create a new collection' do
-      expect(coll.members.size).to eq 0
-      expect_any_instance_of(Collection).to receive(:save!).once
+      expect(actor).to receive(:create).with(hash_including(member_of_collection_ids: [coll.id]))
       expect do
         factory.run
       end.to change { Collection.count }.by(0)
-      expect(coll.reload.members.size).to eq 1
     end
   end
 end

spec/lib/importer/factory/image_factory_spec.rb

@@ -2,7 +2,10 @@
 
 RSpec.describe Importer::Factory::ImageFactory, :clean do
   let(:factory) { described_class.new(attributes) }
-
+  let(:actor) { instance_double(Hyrax::Actors::ActorStack) }
+  before do
+    allow(Hyrax::CurationConcern).to receive(:actor).and_return(actor)
+  end
   let(:files) { [] }
   let(:attributes) do
     {
@@ -19,47 +22,37 @@
   context 'with files' do
     let(:factory) { described_class.new(attributes, 'tmp/files', files) }
     let(:files) { ['img.png'] }
-    let(:file) { double('the file') }
-    let!(:coll) { FactoryGirl.create(:collection) }
-    let(:fs_actor) { double }
-    before do
-      allow(File).to receive(:exist?).and_call_original # For byebug, sprockets, etc
-      allow(File).to receive(:exist?).with("tmp/files/img.png").and_return(true)
-      allow(File).to receive(:new).and_return(file)
-      allow(Hyrax::Actors::FileSetActor).to receive(:new)
-        .with(FileSet, User).and_return(fs_actor)
-    end
+    let!(:coll) { create(:collection) }
+
     context "for a new image" do
       it 'creates file sets with access controls' do
-        expect(fs_actor).to receive(:create_metadata).with(GenericWork)
-        expect(fs_actor).to receive(:create_content).with(file)
+        expect(actor).to receive(:create).with(
+          hash_including(member_of_collection_ids: [coll.id], files: ['tmp/files/img.png'])
+        )
         factory.run
       end
     end
 
     context "for an existing image without files" do
-      let!(:work) { FactoryGirl.create(:generic_work) }
+      let(:work) { create(:generic_work) }
       let(:factory) { described_class.new(attributes.merge(id: work.id), 'tmp/files', files) }
       it 'creates file sets' do
-        expect do
-          expect(fs_actor).to receive(:create_metadata).with(work)
-          expect(fs_actor).to receive(:create_content).with(file)
-          factory.run
-        end.not_to change { GenericWork.count }
+        expect(actor).to receive(:update).with(
+          hash_including(member_of_collection_ids: [coll.id], files: ['tmp/files/img.png'])
+        )
+        factory.run
       end
     end
   end
 
   context 'when a collection already exists' do
-    let!(:coll) { FactoryGirl.create(:collection) }
+    let(:coll) { create(:collection) }
 
     it 'does not create a new collection' do
-      expect(coll.members.size).to eq 0
-      expect_any_instance_of(Collection).to receive(:save!).once
+      expect(actor).to receive(:create).with(hash_including(member_of_collection_ids: [coll.id]))
       expect do
         factory.run
       end.to change { Collection.count }.by(0)
-      expect(coll.reload.members.size).to eq 1
     end
   end
 end

spec/lib/importer/mods_importer_spec.rb

@@ -4,42 +4,39 @@
 RSpec.describe Importer::ModsImporter, :clean do
   let(:image_directory) { 'spec/fixtures/images' }
   let(:importer) { described_class.new(image_directory) }
+  let(:actor) { instance_double(Hyrax::Actors::ActorStack) }
+  before do
+    allow(Hyrax::CurationConcern).to receive(:actor).and_return(actor)
+  end
 
   describe '#import an image' do
     let(:file) { 'spec/fixtures/mods/shpc/druid_xv169dn4538.mods' }
 
     it 'creates a new image and a collection' do
-      image = nil
+      expect(actor).to receive(:create).with(
+        hash_including(:member_of_collection_ids,
+                       identifier: ['xv169dn4538'],
+                       visibility: 'open')
+      )
       expect do
-        image = importer.import(file)
-      end.to change { GenericWork.count }.by(1)
-        .and change { Collection.count }.by(1)
-
-      # Image.reload doesn't clear @file_association
-      reloaded = GenericWork.find(image.id)
-
-      expect(reloaded.identifier).to eq ['xv169dn4538']
-
-      expect(reloaded.read_groups).to eq ['public']
+        importer.import(file)
+      end.to change { Collection.count }.by(1)
 
-      coll = reloaded.in_collections.first
+      coll = Collection.last
       expect(coll.identifier).to eq ['kx532cb7981']
       expect(coll.title).to eq ['Stanford historical photograph collection, 1887-circa 1996']
-      expect(coll.members).to eq [reloaded]
       expect(coll.visibility).to eq 'open'
     end
 
     context 'when the collection already exists' do
-      let!(:coll) { FactoryGirl.create(:collection, id: 'kx532cb7981', title: ['Test Collection']) }
+      before { create(:collection, id: 'kx532cb7981', title: ['Test Collection']) }
 
       it 'it adds image to existing collection' do
-        expect(coll.members.size).to eq 0
+        expect(actor).to receive(:create).with(hash_including(:member_of_collection_ids))
 
         expect do
           importer.import(file)
         end.to change { Collection.count }.by(0)
-
-        expect(coll.reload.members.size).to eq 1
       end
     end
   end