Use the FileMetadata#type not FileSet#original_file_id for use

FileSet's use "relation" should be controlled exclusively by the related FileMetadata's `#type` property. Move toward removing the `original_file_id` and other `FileSet` attributes by reworking the existing queries.
samvera · Jan 31, 2020 · 437dccd · 437dccd
1 parent 838cb01
commit 437dccd
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 126 deletions.
diff --git a/app/services/hyrax/custom_queries/find_file_metadata.rb b/app/services/hyrax/custom_queries/find_file_metadata.rb
@@ -61,6 +61,8 @@ def find_many_file_metadata_by_ids(ids:)
       # @example
       #   Hyrax.query_service.find_file_metadata_by_use(use: ::RDF::URI("http://pcdm.org/ExtractedText"))
       def find_many_file_metadata_by_use(resource:, use:)
+        return [] unless resource.try(:file_ids)
+
         results = find_many_file_metadata_by_ids(ids: resource.file_ids)
         results.select { |fm| fm.type.include?(use) }
       end

diff --git a/app/services/hyrax/custom_queries/navigators/find_files.rb b/app/services/hyrax/custom_queries/navigators/find_files.rb
@@ -1,13 +1,13 @@
 module Hyrax
   module CustomQueries
     module Navigators
+      ##
+      # @example
+      #   Hyrax.custom_queries.find_files(file_set: file_set_resource)
+      #   Hyrax.custom_queries.find_original_file(file_set: file_set_resource)
+      #   Hyrax.custom_queries.find_extracted_text(file_set: file_set_resource)
+      #   Hyrax.custom_queries.find_thumbnail(file_set: file_set_resource)
       class FindFiles
-        # @example
-        #   Hyrax.custom_queries.find_files(file_set: file_set_resource)
-        #   Hyrax.custom_queries.find_original_file(file_set: file_set_resource)
-        #   Hyrax.custom_queries.find_extracted_text(file_set: file_set_resource)
-        #   Hyrax.custom_queries.find_thumbnail(file_set: file_set_resource)
-
         def self.queries
           [:find_files,
            :find_original_file,
@@ -39,40 +39,48 @@ def find_files(file_set:)
         # @param file_set [Hyrax::FileSet]
         # @return [Hyrax::FileMetadata]
         def find_original_file(file_set:)
-          if file_set.respond_to?(:original_file_id)
-            raise ::Valkyrie::Persistence::ObjectNotFoundError, "File set's original file is blank" if file_set.original_file_id.blank?
-            query_service.custom_queries.find_file_metadata_by(id: file_set.original_file_id)
-          else
-            raise ::Valkyrie::Persistence::ObjectNotFoundError,
-                  "#{file_set.internal_resource} is not a `Hydra::FileSet` implementer"
-          end
+          find_exactly_one_file_by_use(
+            file_set: file_set,
+            use: Hyrax::FileMetadata::Use::ORIGINAL_FILE
+          )
         end
 
         # Find extracted text id of a given file set resource, and map to file metadata resource
         # @param file_set [Hyrax::FileSet]
         # @return [Hyrax::FileMetadata]
         def find_extracted_text(file_set:)
-          if file_set.respond_to?(:extracted_text_id)
-            raise ::Valkyrie::Persistence::ObjectNotFoundError, "File set's extracted text is blank" if file_set.extracted_text_id.blank?
-            query_service.custom_queries.find_file_metadata_by(id: file_set.extracted_text_id)
-          else
-            raise ::Valkyrie::Persistence::ObjectNotFoundError,
-                  "#{file_set.internal_resource} is not a `Hydra::FileSet` implementer"
-          end
+          find_exactly_one_file_by_use(
+            file_set: file_set,
+            use: Hyrax::FileMetadata::Use::EXTRACTED_TEXT
+          )
         end
 
         # Find thumbnail id of a given file set resource, and map to file metadata resource
         # @param file_set [Hyrax::FileSet]
         # @return [Hyrax::FileMetadata]
         def find_thumbnail(file_set:)
-          if file_set.respond_to?(:thumbnail_id)
-            raise ::Valkyrie::Persistence::ObjectNotFoundError, "File set's thumbnail is blank" if file_set.thumbnail_id.blank?
-            query_service.custom_queries.find_file_metadata_by(id: file_set.thumbnail_id)
-          else
-            raise ::Valkyrie::Persistence::ObjectNotFoundError,
-                  "#{file_set.internal_resource} is not a `Hydra::FileSet` implementer"
-          end
+          find_exactly_one_file_by_use(
+            file_set: file_set,
+            use: Hyrax::FileMetadata::Use::THUMBNAIL
+          )
         end
+
+        private
+
+          ##
+          # @api private
+          #
+          # @return [Hyrax::FileMetadata]
+          # @raise [Valkyrie::Persistence::ObjectNotFoundError]
+          def find_exactly_one_file_by_use(file_set:, use:)
+            files =
+              query_service.custom_queries.find_many_file_metadata_by_use(resource: file_set, use: use)
+
+            raise Valkyrie::Persistence::ObjectNotFoundError, "FileSet #{file_set.id}'s #{use.fragment} is missing." if
+              files.empty?
+
+            files.first
+          end
       end
     end
   end

diff --git a/lib/hyrax/specs/shared_specs/hydra_works.rb b/lib/hyrax/specs/shared_specs/hydra_works.rb
@@ -215,73 +215,4 @@
       end
     end
   end
-
-  describe 'original file' do
-    it 'has nil original_file_id by default' do
-      expect(fileset.original_file_id).to be_nil
-    end
-
-    context 'with original file' do
-      let(:file_class) { Hyrax::FileMetadata }
-      let(:original_file) { persister.save(resource: file_class.new) }
-      let(:original_file_id) { original_file.id }
-      let(:second_file) { persister.save(resource: file_class.new) }
-
-      before { fileset.original_file_id = original_file_id }
-
-      it 'has original_file_id' do
-        expect(fileset.original_file_id).to eq original_file_id
-      end
-
-      it 'can query original file' do
-        expect(query_service.custom_queries.find_original_file(file_set: fileset)).to eq original_file
-      end
-    end
-  end
-
-  describe 'extracted text' do
-    it 'has nil extracted_text_id by default' do
-      expect(fileset.extracted_text_id).to be_nil
-    end
-
-    context 'with extracted text' do
-      let(:file_class) { Hyrax::FileMetadata }
-      let(:extracted_text) { persister.save(resource: file_class.new) }
-      let(:extracted_text_id) { extracted_text.id }
-      let(:second_file) { persister.save(resource: file_class.new) }
-
-      before { fileset.extracted_text_id = extracted_text_id }
-
-      it 'has extracted_text_id' do
-        expect(fileset.extracted_text_id).to eq extracted_text_id
-      end
-
-      it 'can query extracted text' do
-        expect(query_service.custom_queries.find_extracted_text(file_set: fileset)).to eq extracted_text
-      end
-    end
-  end
-
-  describe 'thumbnail' do
-    it 'has nil thumbnail_id by default' do
-      expect(fileset.thumbnail_id).to be_nil
-    end
-
-    context 'with thumbnail' do
-      let(:file_class) { Hyrax::FileMetadata }
-      let(:thumbnail) { persister.save(resource: file_class.new) }
-      let(:thumbnail_id) { thumbnail.id }
-      let(:second_file) { persister.save(resource: file_class.new) }
-
-      before { fileset.thumbnail_id = thumbnail_id }
-
-      it 'has thumbnail_id' do
-        expect(fileset.thumbnail_id).to eq thumbnail_id
-      end
-
-      it 'can query thumbnail' do
-        expect(query_service.custom_queries.find_thumbnail(file_set: fileset)).to eq thumbnail
-      end
-    end
-  end
 end
diff --git a/lib/wings/services/file_metadata_builder.rb b/lib/wings/services/file_metadata_builder.rb
@@ -57,10 +57,9 @@ def attach_file_metadata_to_valkyrie_file_set(file_metadata, file_set)
       # @api private
       # @return [Hyrax::FileMetadata, nil]
       def current_original_file(file_set)
-        Hyrax.query_service
-             .custom_queries
-             .find_many_file_metadata_by_use(resource: file_set, use: Hyrax::FileMetadata::Use::ORIGINAL_FILE)
-             .first
+        Hyrax.custom_queries.find_original_file(file_set: file_set)
+      rescue ::Valkyrie::Persistence::ObjectNotFoundError
+        nil
       end
 
       # Class for wrapping the file being ingested

diff --git a/spec/services/hyrax/custom_queries/navigators/find_files_spec.rb b/spec/services/hyrax/custom_queries/navigators/find_files_spec.rb
@@ -5,16 +5,19 @@
 
   describe '#find_files' do
     context 'when files exist' do
-      let!(:file_metadata1) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
-      let!(:file_metadata2) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
-      let!(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [file_metadata1, file_metadata2]) }
+      let(:file_metadata1) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
+      let(:file_metadata2) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
+      let(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [file_metadata1, file_metadata2]) }
+
       it 'returns file metadata resource' do
-        expect(query_handler.find_files(file_set: fileset).map(&:id).map(&:to_s)).to match_array [file_metadata1.id.to_s, file_metadata2.id.to_s]
+        expect(query_handler.find_files(file_set: fileset).map(&:id).map(&:to_s))
+          .to match_array [file_metadata1.id.to_s, file_metadata2.id.to_s]
       end
     end
 
     context 'when files do not exist' do
-      let!(:fileset) { FactoryBot.build(:hyrax_file_set) }
+      let(:fileset) { FactoryBot.build(:hyrax_file_set) }
+
       it 'returns an empty array' do
         expect(query_handler.find_files(file_set: fileset)).to be_empty
       end
@@ -23,78 +26,93 @@
 
   describe '#find_original_file' do
     context 'when original file exists' do
-      let!(:original_file) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
-      let!(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [original_file], original_file: original_file) }
+      let(:original_file) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
+      let(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [original_file]) }
+
       it 'returns file metadata resource' do
         expect(query_handler.find_original_file(file_set: fileset).id.to_s).to eq original_file.id.to_s
       end
     end
 
     context 'when files do not exist' do
-      let!(:fileset) { FactoryBot.build(:hyrax_file_set) }
+      let(:fileset) { FactoryBot.build(:hyrax_file_set) }
+
       it 'raises error' do
         expect { query_handler.find_original_file(file_set: fileset) }
-          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError, "File set's original file is blank"
+          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
       end
     end
 
-    context 'when file_set does not respond to original file' do
-      let!(:fileset) { FactoryBot.build(:hyrax_resource) }
+    context 'when resource does not respond file_ids' do
+      let(:fileset) { FactoryBot.build(:hyrax_resource) }
+
       it 'raises error' do
         expect { query_handler.find_original_file(file_set: fileset) }
-          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError, "Hyrax::Resource is not a `Hydra::FileSet` implementer"
+          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
       end
     end
   end
 
   describe '#find_extracted_text' do
     context 'when extracted text exists' do
-      let!(:extracted_text) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
-      let!(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [extracted_text], extracted_text: extracted_text) }
+      let(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [extracted_text]) }
+
+      let(:extracted_text) do
+        FactoryBot.valkyrie_create(:hyrax_file_metadata, type: Hyrax::FileMetadata::Use::EXTRACTED_TEXT)
+      end
+
       it 'returns file metadata resource' do
         expect(query_handler.find_extracted_text(file_set: fileset).id.to_s).to eq extracted_text.id.to_s
       end
     end
 
     context 'when files do not exist' do
-      let!(:fileset) { FactoryBot.build(:hyrax_file_set) }
+      let(:fileset) { FactoryBot.build(:hyrax_file_set) }
+
       it 'raises error' do
         expect { query_handler.find_extracted_text(file_set: fileset) }
-          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError, "File set's extracted text is blank"
+          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
       end
     end
 
-    context 'when file_set does not respond to extracted text' do
-      let!(:fileset) { FactoryBot.build(:hyrax_resource) }
+    context 'when resource does not respond file_ids' do
+      let(:fileset) { FactoryBot.build(:hyrax_resource) }
+
       it 'raises error' do
         expect { query_handler.find_extracted_text(file_set: fileset) }
-          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError, "Hyrax::Resource is not a `Hydra::FileSet` implementer"
+          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
       end
     end
   end
 
   describe '#find_thumbnail' do
     context 'when thumbnail exists' do
-      let!(:thumbnail) { FactoryBot.valkyrie_create(:hyrax_file_metadata) }
-      let!(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [thumbnail], thumbnail: thumbnail) }
+      let(:fileset) { FactoryBot.valkyrie_create(:hyrax_file_set, files: [thumbnail], thumbnail: thumbnail) }
+
+      let(:thumbnail) do
+        FactoryBot.valkyrie_create(:hyrax_file_metadata, type: Hyrax::FileMetadata::Use::THUMBNAIL)
+      end
+
       it 'returns file metadata resource' do
         expect(query_handler.find_thumbnail(file_set: fileset).id.to_s).to eq thumbnail.id.to_s
       end
     end
 
     context 'when files do not exist' do
-      let!(:fileset) { FactoryBot.build(:hyrax_file_set) }
+      let(:fileset) { FactoryBot.build(:hyrax_file_set) }
+
       it 'raises error' do
         expect { query_handler.find_thumbnail(file_set: fileset) }
-          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError, "File set's thumbnail is blank"
+          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
       end
     end
 
-    context 'when file_set does not respond to thumbnail' do
-      let!(:fileset) { FactoryBot.build(:hyrax_resource) }
+    context 'when resource does not respond file_ids' do
+      let(:fileset) { FactoryBot.build(:hyrax_resource) }
+
       it 'raises error' do
         expect { query_handler.find_thumbnail(file_set: fileset) }
-          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError, "Hyrax::Resource is not a `Hydra::FileSet` implementer"
+          .to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
       end
     end
   end