Update callbacks with upstream improvements

lib/active_fedora.rb

@@ -71,6 +71,7 @@ module ActiveFedora #:nodoc:
     autoload :File
     autoload :FileConfigurator
     autoload :FilePathBuilder
+    autoload :FilePersistence
     autoload :FileRelation
     autoload :FilesHash
     autoload :FixityService

lib/active_fedora/associations/collection_association.rb

@@ -212,15 +212,15 @@ def create(attrs = {})
         if attrs.is_a?(Array)
           attrs.collect { |attr| create(attr) }
         else
-          create_record(attrs) do |record|
+          _create_record(attrs) do |record|
             yield(record) if block_given?
             record.save
           end
         end
       end
 
       def create!(attrs = {})
-        create_record(attrs) do |record|
+        _create_record(attrs) do |record|
           yield(record) if block_given?
           record.save!
         end
@@ -340,7 +340,7 @@ def find_reflection
           end
         end
 
-        def create_record(attributes, raise = false)
+        def _create_record(attributes, raise = false)
           attributes.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
           ensure_owner_is_not_new
 

lib/active_fedora/callbacks.rb

@@ -227,21 +227,21 @@ module Callbacks
     end
 
     def destroy(*) #:nodoc:
-      run_callbacks(:destroy) { super }
+      _run_destroy_callbacks { super }
     end
 
     private
 
-      def create_record(*) #:nodoc:
-        run_callbacks(:create) do
-          run_callbacks(:save) { super }
-        end
+      def create_or_update(*)
+        _run_save_callbacks { super }
       end
 
-      def update_record(*) #:nodoc:
-        run_callbacks(:update) do
-          run_callbacks(:save) { super }
-        end
+      def _create_record(*) #:nodoc:
+        _run_create_callbacks { super }
+      end
+
+      def _update_record(*) #:nodoc:
+        _run_update_callbacks { super }
       end
   end
 end

lib/active_fedora/errors.rb

@@ -44,6 +44,25 @@ class UnregisteredPredicateError < ActiveFedoraError
   class RecordNotSaved < ActiveFedoraError
   end
 
+  # Raised by {ActiveFedora::Base#destroy!}[rdoc-ref:Persistence#destroy!]
+  # when a call to {#destroy}[rdoc-ref:Persistence#destroy!]
+  # would return false.
+  #
+  #   begin
+  #     complex_operation_that_internally_calls_destroy!
+  #   rescue ActiveFedora::RecordNotDestroyed => invalid
+  #     puts invalid.record.errors
+  #   end
+  #
+  class RecordNotDestroyed < ActiveFedoraError
+    attr_reader :record
+
+    def initialize(message = nil, record = nil)
+      @record = record
+      super(message)
+    end
+  end
+
   # Raised on attempt to update record that is instantiated as read only.
   class ReadOnlyRecord < ActiveFedoraError
   end

lib/active_fedora/file.rb

@@ -14,9 +14,10 @@ class File
 
     include ActiveFedora::File::Attributes
     include ActiveFedora::File::Streaming
-    include ActiveFedora::Persistence
+    include ActiveFedora::FilePersistence
     include ActiveFedora::Versionable
     include ActiveModel::Dirty
+    include ActiveFedora::Callbacks
     include AttributeMethods # allows 'content' to be tracked
     include Identifiable
     include Scoping
@@ -190,10 +191,6 @@ def readonly?
       false
     end
 
-    def destroy(*)
-      run_callbacks(:destroy) { super }
-    end
-
     def self.relation
       FileRelation.new(self)
     end
@@ -224,32 +221,6 @@ def ldp_headers
         headers
       end
 
-      def create_record(_options = {})
-        run_callbacks(:create) do
-          run_callbacks(:save) do
-            return false if content.nil?
-            ldp_source.content = content
-            ldp_source.create do |req|
-              req.headers.merge!(ldp_headers)
-            end
-            refresh
-          end
-        end
-      end
-
-      def update_record(_options = {})
-        run_callbacks(:update) do
-          run_callbacks(:save) do
-            return true unless content_changed?
-            ldp_source.content = content
-            ldp_source.update do |req|
-              req.headers.merge!(ldp_headers)
-            end
-            refresh
-          end
-        end
-      end
-
       def build_ldp_resource(id)
         build_ldp_resource_via_uri self.class.id_to_uri(id)
       end

lib/active_fedora/file_persistence.rb

@@ -0,0 +1,27 @@
+module ActiveFedora
+  module FilePersistence
+    extend ActiveSupport::Concern
+
+    include ActiveFedora::Persistence
+
+    private
+
+      def _create_record(_options = {})
+        return false if content.nil?
+        ldp_source.content = content
+        ldp_source.create do |req|
+          req.headers.merge!(ldp_headers)
+        end
+        refresh
+      end
+
+      def _update_record(_options = {})
+        return true unless content_changed?
+        ldp_source.content = content
+        ldp_source.update do |req|
+          req.headers.merge!(ldp_headers)
+        end
+        refresh
+      end
+  end
+end

lib/active_fedora/indexing.rb

@@ -59,14 +59,14 @@ def update_needs_index?
     private
 
       # index the record after it has been persisted to Fedora
-      def create_record(options = {})
+      def _create_record(options = {})
         super
         update_index if create_needs_index? && options.fetch(:update_index, true)
         true
       end
 
       # index the record after it has been updated in Fedora
-      def update_record(options = {})
+      def _update_record(options = {})
         super
         update_index if update_needs_index? && options.fetch(:update_index, true)
         true

lib/active_fedora/persistence.rb

@@ -70,6 +70,17 @@ def destroy(*opts)
       delete(*opts)
     end
 
+    # Deletes the record in the database and freezes this instance to reflect
+    # that no changes should be made (since they can't be persisted).
+    #
+    # There's a series of callbacks associated with #destroy!. If the
+    # <tt>before_destroy</tt> callback throws +:abort+ the action is cancelled
+    # and #destroy! raises ActiveFedora::RecordNotDestroyed.
+    # See ActiveFedora::Callbacks for further details.
+    def destroy!
+      destroy || _raise_record_not_destroyed
+    end
+
     def eradicate
       self.class.eradicate(id)
     end
@@ -138,12 +149,12 @@ def delete_tombstone(uri)
 
       def create_or_update(*args)
         raise ReadOnlyRecord if readonly?
-        result = new_record? ? create_record(*args) : update_record(*args)
+        result = new_record? ? _create_record(*args) : _update_record(*args)
         result != false
       end
 
       # Deals with preparing new object to be saved to Fedora, then pushes it and its attached files into Fedora.
-      def create_record(_options = {})
+      def _create_record(_options = {})
         assign_rdf_subject
         serialize_attached_files
         @ldp_source = @ldp_source.create
@@ -152,13 +163,20 @@ def create_record(_options = {})
         refresh
       end
 
-      def update_record(_options = {})
+      def _update_record(_options = {})
         serialize_attached_files
         execute_sparql_update
         save_contained_resources
         refresh
       end
 
+      def _raise_record_not_destroyed
+        @_association_destroy_exception ||= nil
+        raise @_association_destroy_exception || RecordNotDestroyed.new("Failed to destroy the record", self)
+      ensure
+        @_association_destroy_exception = nil
+      end
+
       def refresh
         @ldp_source = build_ldp_resource(id)
         @resource = nil

spec/integration/file_spec.rb

@@ -108,7 +108,7 @@ class MockAFBase < ActiveFedora::Base
         test_object.save
       end
 
-      it "does not be changed" do
+      it "is not changed" do
         expect(test_object.attached_files[path]).to_not be_changed
       end
 

spec/unit/file_spec.rb

@@ -379,9 +379,9 @@ def a_update; end
           subject.content = "foo"
           subject.save
           expect(subject).to receive(:b_save).once
-          expect(subject).not_to receive(:a_save)
+          expect(subject).to receive(:a_save)
           expect(subject).to receive(:b_update).once
-          expect(subject).not_to receive(:a_update)
+          expect(subject).to receive(:a_update)
           subject.save
         }
       end

spec/unit/validations_spec.rb

@@ -47,7 +47,7 @@ class ValidationStub < ActiveFedora::Base
   end
 
   describe "#save!" do
-    before { allow(subject).to receive(:create_record) } # prevent saving to Fedora/Solr
+    before { allow(subject).to receive(:_create_record) } # prevent saving to Fedora/Solr
 
     it "validates only once" do
       expect(subject).to receive(:perform_validations).once.and_return(true)