From 85d1a0875f61c3dbe00f8bc4da2a4fcdbd550d5a Mon Sep 17 00:00:00 2001 From: "Michael J. Giarlo" Date: Fri, 20 May 2016 16:32:45 -0700 Subject: [PATCH] ActiveFedora::File.mime_type should be updatable. Fixes #890, #1083 This unreverts a commit by @cjcolvar: https://github.com/projecthydra/active_fedora/pull/1035/commits/cd865dec0203ed79ad36c4b40d5dd7c2bd943d3c Which was itself a squash and rebase of work that @tpendragon did. Note that there are still two failing specs. Feel free to add commits to this branch and march this over the finish line. The two failures: ``` 1) ActiveFedora::WithMetadata properties with defaults should respond to #has_mime_type Failure/Error: it { is_expected.to respond_to(:has_mime_type) } expected # to respond to :has_mime_type # ./spec/integration/with_metadata_spec.rb:46:in `block (4 levels) in ' 2) ActiveFedora::WithMetadata::DefaultMetadataClassFactory::build sets MetadataNode to the default schema using the default strategy Failure/Error: expect(parent).to receive(:delegate).at_least(8).times (Double "Parent").delegate(*(any args)) expected: at least 8 times with any arguments received: 7 times with any arguments # ./spec/unit/with_metadata/default_metadata_class_factory_spec.rb:15:in `block (3 levels) in ' Finished in 3 minutes 41.7 seconds (files took 1.65 seconds to load) 1065 examples, 2 failures, 1 pending ``` --- README.md | 3 +-- lib/active_fedora/file.rb | 20 +++++++++++++------ lib/active_fedora/file/attributes.rb | 10 +++++----- .../with_metadata/default_schema.rb | 1 - .../with_metadata/metadata_node.rb | 8 +++++++- spec/integration/file_spec.rb | 2 +- spec/integration/om_datastream_spec.rb | 16 +++++++-------- spec/integration/with_metadata_spec.rb | 2 +- spec/unit/file_spec.rb | 19 ++++++++++++++++++ .../default_metadata_class_factory_spec.rb | 11 +++++++++- 10 files changed, 66 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 55de903b4..b0a041eb0 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Description ActiveFedora is a Ruby gem for creating and managing objects in the Fedora Repository Architecture ([http://fedora-commons.org](http://fedora-commons.org)). ActiveFedora -is loosely based on “ActiveRecord” in Rails. Version 9.0+ works with Fedora 4 and prior versions work on Fedora 3. Version 9.2+ works with Solr 4.10. +is loosely based on “ActiveRecord” in Rails. Version 9.0+ works with Fedora 4 and prior versions work on Fedora 3. Version 9.2+ works with Solr 4.10. Version 10.0+ works with Fedora >= 4.5.1. Getting Help ------------ @@ -101,4 +101,3 @@ Creator: Matt Zumwalt ([MediaShelf](http://yourmediashelf.com)) Developers: Justin Coyne, McClain Looney & Eddie Shin ([MediaShelf](http://yourmediashelf.com)), Rick Johnson (Notre Dame) - diff --git a/lib/active_fedora/file.rb b/lib/active_fedora/file.rb index e0532bddd..7d23a26bd 100644 --- a/lib/active_fedora/file.rb +++ b/lib/active_fedora/file.rb @@ -50,6 +50,12 @@ def initialize(identifier = nil, &_block) end end + def save + super.tap do + metadata.save if metadata.changed? + end + end + def described_by raise "#{self} isn't persisted yet" if new_record? links['describedby'].first @@ -99,11 +105,8 @@ def datastream_will_change! end def attribute_will_change!(attr) - if attr == 'content' - changed_attributes['content'] = true - else - super - end + return super unless attr == 'content' + changed_attributes['content'] = true end def remote_content @@ -124,8 +127,13 @@ def content_changed? local_or_remote_content(false) != @ds_content end + def metadata_changed? + return false if new_record? || links['describedby'].blank? + metadata.changed? + end + def changed? - super || content_changed? + super || content_changed? || metadata_changed? end def inspect diff --git a/lib/active_fedora/file/attributes.rb b/lib/active_fedora/file/attributes.rb index 5e1deebd3..9e1418514 100644 --- a/lib/active_fedora/file/attributes.rb +++ b/lib/active_fedora/file/attributes.rb @@ -1,12 +1,12 @@ module ActiveFedora::File::Attributes - attr_writer :mime_type + delegate :mime_type=, to: :metadata def assign_attributes(_) # nop end def mime_type - @mime_type ||= fetch_mime_type + fetch_mime_type end def original_name @@ -29,7 +29,7 @@ def persisted_size end def dirty_size - content.size if changed? && content.respond_to?(:size) + content.size if content_changed? && content.respond_to?(:size) end def size @@ -72,8 +72,8 @@ def default_mime_type end def fetch_mime_type - return default_mime_type if new_record? - ldp_source.head.content_type + return default_mime_type if new_record? && metadata.mime_type.blank? + metadata.mime_type.first end def fetch_original_name diff --git a/lib/active_fedora/with_metadata/default_schema.rb b/lib/active_fedora/with_metadata/default_schema.rb index cdde7989e..bd8461671 100644 --- a/lib/active_fedora/with_metadata/default_schema.rb +++ b/lib/active_fedora/with_metadata/default_schema.rb @@ -7,7 +7,6 @@ class DefaultSchema < ActiveTriples::Schema property :file_name, predicate: ::RDF::Vocab::EBUCore.filename property :file_size, predicate: ::RDF::Vocab::EBUCore.fileSize property :date_created, predicate: ::RDF::Vocab::EBUCore.dateCreated - property :has_mime_type, predicate: ::RDF::Vocab::EBUCore.hasMimeType property :date_modified, predicate: ::RDF::Vocab::EBUCore.dateModified property :byte_order, predicate: SweetJPLTerms.byteOrder # This is a server-managed predicate which means Fedora does not let us change it. diff --git a/lib/active_fedora/with_metadata/metadata_node.rb b/lib/active_fedora/with_metadata/metadata_node.rb index f4bd72231..4f73d5995 100644 --- a/lib/active_fedora/with_metadata/metadata_node.rb +++ b/lib/active_fedora/with_metadata/metadata_node.rb @@ -4,6 +4,12 @@ class MetadataNode < ActiveTriples::Resource include ActiveModel::Dirty attr_reader :file + # mime_type is treated differently than all the other metadata properties, + # in that it can be set at file-create time (as a HTTP post header) and be + # updated (using SPARQL update) on the metadata-node. Therefore, it is in + # this class rather than being in the DefaultSchema. + property :mime_type, predicate: ::RDF::Vocab::EBUCore.hasMimeType + def initialize(file) @file = file super(file.uri, ldp_source.graph) @@ -45,7 +51,7 @@ def save def changed_attributes super.tap do |changed| - changed['type'] = true if type.present? + changed['type'] = true if type.present? && new_record? end end diff --git a/spec/integration/file_spec.rb b/spec/integration/file_spec.rb index 5c862e756..d449f8d5c 100644 --- a/spec/integration/file_spec.rb +++ b/spec/integration/file_spec.rb @@ -122,7 +122,7 @@ class MockAFBase < ActiveFedora::Base end it "is not changed" do - expect(test_object.attached_files[path]).to_not be_changed + expect(test_object.attached_files[path]).to_not be_content_changed end it "is able to read the content from fedora" do diff --git a/spec/integration/om_datastream_spec.rb b/spec/integration/om_datastream_spec.rb index 39be751b3..5069cdf8e 100644 --- a/spec/integration/om_datastream_spec.rb +++ b/spec/integration/om_datastream_spec.rb @@ -21,24 +21,24 @@ class ModsArticle2 < ActiveFedora::Base subject { obj.descMetadata } describe "#changed?" do - it "does not be changed when no fields have been set" do + it "is not changed when no fields have been set" do expect(subject).to_not be_content_changed end it "is changed when a field has been set" do subject.title = 'Foobar' expect(subject).to be_content_changed end - it "does not be changed if the new xml matches the old xml" do + it "is not changed if the new xml matches the old xml" do subject.content = subject.content - expect(subject).to_not be_changed + expect(subject).to_not be_content_changed end it "is changed if there are minor differences in whitespace" do subject.content = "1" obj.save - expect(subject).to_not be_changed + expect(subject).to_not be_content_changed subject.content = "\n1\n" - expect(subject).to be_changed + expect(subject).to be_content_changed end end @@ -56,11 +56,11 @@ class ModsArticle2 < ActiveFedora::Base obj.reload end - it "does not be dirty after .update_values is saved" do + it "is not dirty after .update_values is saved" do obj.descMetadata.update_values([{ name: 0 }, { role: 0 }, :text] => "Funder") - expect(obj.descMetadata).to be_changed + expect(obj.descMetadata).to be_content_changed obj.save - expect(obj.descMetadata).to_not be_changed + expect(obj.descMetadata).to_not be_content_changed expect(obj.descMetadata.term_values({ name: 0 }, { role: 0 }, :text)).to eq ["Funder"] end end diff --git a/spec/integration/with_metadata_spec.rb b/spec/integration/with_metadata_spec.rb index 2f54b7684..a85aed530 100644 --- a/spec/integration/with_metadata_spec.rb +++ b/spec/integration/with_metadata_spec.rb @@ -43,7 +43,7 @@ class SampleFile < ActiveFedora::File it { is_expected.to respond_to(:file_name) } it { is_expected.to respond_to(:file_size) } it { is_expected.to respond_to(:date_created) } - it { is_expected.to respond_to(:has_mime_type) } + it { is_expected.to respond_to(:mime_type) } it { is_expected.to respond_to(:date_modified) } it { is_expected.to respond_to(:byte_order) } it { is_expected.to respond_to(:file_hash) } diff --git a/spec/unit/file_spec.rb b/spec/unit/file_spec.rb index 90ad8239c..6ce15b4bf 100644 --- a/spec/unit/file_spec.rb +++ b/spec/unit/file_spec.rb @@ -179,6 +179,25 @@ end end + describe "#mime_type" do + let(:parent) { ActiveFedora::Base.create } + before do + parent.add_file('banana', path: 'apple', mime_type: 'video/webm') + parent.save! + end + it "persists" do + expect(parent.reload.apple.mime_type).to eq "video/webm" + end + it "can be updated" do + parent.reload + parent.apple.mime_type = "text/awesome" + expect(parent.apple.mime_type).to eq "text/awesome" + parent.save + + expect(parent.reload.apple.mime_type).to eq "text/awesome" + end + end + context "original_name" do subject { file.original_name } diff --git a/spec/unit/with_metadata/default_metadata_class_factory_spec.rb b/spec/unit/with_metadata/default_metadata_class_factory_spec.rb index a86770d09..02efd670b 100644 --- a/spec/unit/with_metadata/default_metadata_class_factory_spec.rb +++ b/spec/unit/with_metadata/default_metadata_class_factory_spec.rb @@ -12,7 +12,16 @@ describe "::build" do it "sets MetadataNode to the default schema using the default strategy" do expect(parent).to receive(:const_set) - expect(parent).to receive(:delegate).at_least(8).times + expect(parent).to receive(:delegate).with(:label, :label=, :label_changed?, to: :metadata_node) + expect(parent).to receive(:delegate).with(:file_name, :file_name=, :file_name_changed?, to: :metadata_node) + expect(parent).to receive(:delegate).with(:file_size, :file_size=, :file_size_changed?, to: :metadata_node) + expect(parent).to receive(:delegate).with(:date_created, :date_created=, :date_created_changed?, to: :metadata_node) + expect(parent).to receive(:delegate).with(:date_modified, + :date_modified=, + :date_modified_changed?, + to: :metadata_node) + expect(parent).to receive(:delegate).with(:byte_order, :byte_order=, :byte_order_changed?, to: :metadata_node) + expect(parent).to receive(:delegate).with(:file_hash, :file_hash=, :file_hash_changed?, to: :metadata_node) subject.class.build(parent) end end