Collections should not allow FileSets as members. Fixes #257

lib/hydra/works.rb

@@ -23,6 +23,7 @@ module Vocab
     end
 
     autoload :Characterization
+    autoload :NotFileSetValidator
 
     autoload_under 'models' do
       autoload :Collection

lib/hydra/works/models/concerns/collection_behavior.rb

@@ -15,10 +15,16 @@ module Hydra::Works
   #   9) Hydra::Works::Collection can have access metadata
   module CollectionBehavior
     extend ActiveSupport::Concern
-    include Hydra::PCDM::CollectionBehavior
 
     included do
+      def self.type_validator
+        Hydra::PCDM::Validators::CompositeValidator.new(
+          super,
+          Hydra::Works::NotFileSetValidator
+        )
+      end
       type [Hydra::PCDM::Vocab::PCDMTerms.Collection, Vocab::WorksTerms.Collection]
+      include Hydra::PCDM::CollectionBehavior
     end
 
     def ordered_works

lib/hydra/works/not_file_set_validator.rb

@@ -0,0 +1,9 @@
+module Hydra::Works
+  class NotFileSetValidator
+    def self.validate!(_association, record)
+      if record.try(:file_set?)
+        raise ActiveFedora::AssociationTypeMismatch, "#{record} is a FileSet and may not be a member of the association"
+      end
+    end
+  end
+end

lib/hydra/works/services/add_file_to_file_set.rb

@@ -56,15 +56,10 @@ def attach_attributes(file)
         # Return mime_type based on methods available to file
         # @param object for mimetype to be determined. Attempts to use methods: :mime_type, :content_type, and :path.
         def determine_mime_type(file)
-          if file.respond_to? :mime_type
-            file.mime_type
-          elsif file.respond_to? :content_type
-            file.content_type
-          elsif file.respond_to? :path
-            Hydra::PCDM::GetMimeTypeForFile.call(file.path)
-          else
-            'application/octet-stream'
-          end
+          return file.mime_type if file.respond_to? :mime_type
+          return file.content_type if file.respond_to? :content_type
+          return Hydra::PCDM::GetMimeTypeForFile.call(file.path) if file.respond_to? :path
+          'application/octet-stream'
         end
 
         # Return original_name based on methods available to file

spec/hydra/works/models/collection_spec.rb

@@ -2,7 +2,6 @@
 
 describe Hydra::Works::Collection do
   let(:collection) { described_class.new }
-
   let(:collection1) { described_class.new }
   let(:work1) { Hydra::Works::Work.new }
 
@@ -110,4 +109,23 @@
     subject { collection.in_collections }
     it { is_expected.to eq [collection1] }
   end
+
+  describe 'adding file_sets to collections' do
+    let(:file_set) { Hydra::Works::FileSet.new }
+    let(:exception) { ActiveFedora::AssociationTypeMismatch }
+    context 'with ordered members' do
+      it 'raises AssociationTypeMismatch' do
+        expect { collection.ordered_members = [file_set] }.to raise_error(exception)
+        expect { collection.ordered_members += [file_set] }.to raise_error(exception)
+        expect { collection.ordered_members << file_set }.to raise_error(exception)
+      end
+    end
+    context 'with unordered members' do
+      it 'raises AssociationTypeMismatch' do
+        expect { collection.members = [file_set] }.to raise_error(exception)
+        expect { collection.members += [file_set] }.to raise_error(exception)
+        expect { collection.members << file_set }.to raise_error(exception)
+      end
+    end
+  end
 end

spec/hydra/works/services/persist_derivatives_spec.rb

@@ -31,7 +31,7 @@
       end
 
       mock_add_file_to_file_set(file_set, file)
-      allow_any_instance_of(Hydra::Works::FileSet).to receive(:mime_type).and_return(mime_type)
+      allow(file).to receive(:mime_type).and_return(mime_type)
       # Mock .save to permit tests to run without hitting fedora persistence layer
       allow(file_set).to receive(:save).and_return(file_set)
     end