Cleanup comment cruft and style considerations

app/jobs/characterize_job.rb

@@ -1,10 +1,9 @@
 # Characterizes the file at 'filepath' if available, otherwise, pulls a copy from the repository
 # and runs characterization on that file.
-
 class CharacterizeJob < Hyrax::ApplicationJob
   queue_as Hyrax.config.ingest_queue_name
 
-  def perform(work)
-    Hyrax::Characterizer.for(source: work).characterize
+  def perform(file_set)
+    Hyrax::Characterizer.for(source: file_set).characterize
   end
 end

app/services/hyrax/characterizer.rb

@@ -1,12 +1,14 @@
+# frozen_string_literal: true
+
 module Hyrax
   ##
-  # @abstract Propogates visibility from a provided object (e.g. a Work) to some
-  # group of its members (e.g. file_sets).
+  # Determines which characterizer to run based on the file_set type
+  # allowing implementation of Valkyrie file_sets
   class Characterizer
     ##
-    # @param source [#visibility] the object to propogate visibility from
+    # @param source: the object to run a characterizer on
     #
-    # @return [#propogate]
+    # @return [#characterize]
     def self.for(source:)
       case source
       when Hyrax::FileSetBehavior # ActiveFedora

app/services/hyrax/file_set_characterizer.rb

@@ -2,39 +2,34 @@
 
 module Hyrax
   ##
-  # Propogates visibility from a given Work to its FileSets
+  # Characterizes an ActiveFedora based FileSet
   class FileSetCharacterizer
     ##
     # @!attribute [rw] source
-    #   @return [#visibility]
+    #   @return [#characterize]
     attr_accessor :source
 
     ##
-    # @param source [#visibility] the object to propogate visibility from
+    # @param source the object to characterize
     def initialize(source:)
-      self.source = source
+      @source = source
     end
 
     ##
     # @return [void]
     #
-    # @raise [RuntimeError] if visibility propogation fails
+    # @raise [RuntimeError] if FileSet is missing the characterization_proxy
     def characterize
       Hydra::Works::CharacterizationService.run(characterization_proxy, filepath)
       Rails.logger.debug "Ran characterization on #{characterization_proxy.id} (#{characterization_proxy.mime_type})"
       characterization_proxy.alpha_channels = channels(filepath) if source.image? && Hyrax.config.iiif_image_server?
       characterization_proxy.save!
-      update_source
+      source.update_index
       CreateDerivativesJob.perform_later(source, source.original_file.id, filepath)
     end
 
     private
 
-      def update_source
-        source.update_index
-        source.parent&.in_collections&.each(&:update_index)
-      end
-
       def characterization_proxy
         raise "#{source.class.characterization_proxy} was not found for FileSet #{source.id}" unless source.characterization_proxy?
         source.characterization_proxy

app/services/hyrax/resource_characterizer.rb

@@ -2,44 +2,42 @@
 
 module Hyrax
   ##
-  # Propogates visibility from a given Work to its FileSets
+  # Characterizes a Valkyrie based FileSet
   class ResourceCharacterizer
     ##
     # @!attribute [rw] source
-    #   @return [#visibility]
+    #   @return [#characterize]
     attr_accessor :source
 
     ##
-    # @param source [#visibility] the object to propogate visibility from
+    # @param source the object to characterize
     def initialize(source:)
-      self.source = source
+      @source = source
     end
 
     ##
     # @return [void]
     #
-    # @raise [RuntimeError] if visibility propogation fails
+    # @raise [RuntimeError] if FileSet is missing the characterization_proxy
     def characterize
       Hydra::Works::CharacterizationService.run(characterization_proxy, filepath)
       Rails.logger.debug "Ran characterization on #{characterization_proxy.id} (#{characterization_proxy.mime_type})"
       characterization_proxy.alpha_channels = channels(filepath) if source.image? && Hyrax.config.iiif_image_server?
-      persist
+      Hyrax.persister.save(resource: characterization_proxy)
+      Hyrax.persister.save(resource: source)
       CreateDerivativesJob.perform_later(source, source.original_file.id, filepath)
     end
 
     private
 
-      def persist
-        Hyrax.persister.save(resource: characterization_proxy)
-        Hyrax.persister.save(resource: source)
-      end
-
       def characterization_proxy
         raise "#{source.class.characterization_proxy} was not found for FileSet #{source.id}" unless source.characterization_proxy?
         source.characterization_proxy
       end
 
       def filepath
+        # The current version of Valkyrie id returns a Valkyrie::ID and requires a .id to actually retrieve the id.
+        # This should be updated to source.id after a Valkyrie update
         Hyrax::WorkingDirectory.find_or_retrieve(source.original_file.id, source.id.id)
       end
 

spec/jobs/characterize_job_spec.rb

@@ -1,34 +1,32 @@
 RSpec.describe CharacterizeJob do
-  context 'when the work is an ActiveFedora FileSet' do
-    let(:file_set_id) { 'abc12345' }
-    let(:filename)    { Rails.root.join('tmp', 'uploads', 'ab', 'c1', '23', '45', 'abc12345', 'picture.png').to_s }
-    let(:file_set) do
-      FileSet.new(id: file_set_id).tap do |fs|
-        allow(fs).to receive(:original_file).and_return(file)
-        allow(fs).to receive(:update_index)
-      end
+  let(:file_set_id) { 'abc12345' }
+  let(:filename)    { Rails.root.join('tmp', 'uploads', 'ab', 'c1', '23', '45', 'abc12345', 'picture.png').to_s }
+  let(:file_set) do
+    FileSet.new(id: file_set_id).tap do |fs|
+      allow(fs).to receive(:original_file).and_return(file)
+      allow(fs).to receive(:update_index)
     end
-    let(:file) do
-      Hydra::PCDM::File.new.tap do |f|
-        f.content = 'foo'
-        f.original_name = 'picture.png'
-        f.save!
-        allow(f).to receive(:save!)
-      end
+  end
+  let(:file) do
+    Hydra::PCDM::File.new.tap do |f|
+      f.content = 'foo'
+      f.original_name = 'picture.png'
+      f.save!
+      allow(f).to receive(:save!)
     end
+  end
 
-    context "when the file set's work is in a collection" do
-      let(:work)       { build(:generic_work) }
-      let(:collection) { build(:collection_lw) }
+  context "when the file set's work is in a collection" do
+    let(:work)       { build(:generic_work) }
+    let(:collection) { build(:collection_lw) }
 
-      before do
-        allow(file_set).to receive(:parent).and_return(work)
-        allow(work).to receive(:in_collections).and_return([collection])
-      end
-      it "reindexes the collection" do
-        expect(collection).to receive(:update_index)
-        described_class.perform_now(file_set)
-      end
+    before do
+      allow(file_set).to receive(:parent).and_return(work)
+      allow(work).to receive(:in_collections).and_return([collection])
+    end
+    it "reindexes the collection" do
+      expect(collection).to receive(:update_index)
+      described_class.perform_now(file_set)
     end
   end
 end

spec/services/hyrax/file_set_characterizer_spec.rb

@@ -1,5 +1,6 @@
 RSpec.describe Hyrax::FileSetCharacterizer do
   subject(:characterizer) { described_class.new(source: file_set) }
+
   let(:file_set_id) { 'abc12345' }
   let(:filename)    { Rails.root.join('tmp', 'uploads', 'ab', 'c1', '23', '45', 'abc12345', 'picture.png').to_s }
   let(:file_set) do

spec/services/hyrax/resource_characterizer_spec.rb

@@ -1,6 +1,5 @@
 RSpec.describe Hyrax::ResourceCharacterizer do
   subject(:characterizer) { described_class.new(source: resource) }
-  let(:resource) { FactoryBot.create(:file_set).valkyrie_resource }
 
   before do
     allow(Hyrax::FileSet).to receive(:find).with(resource.id).and_return(resource)