Separate the indexing concerns out of the Persistence module

lib/active_fedora/base.rb

@@ -28,6 +28,7 @@ class Base
 
     include Core
     include Persistence
+    include Indexing
     include Scoping
     include ActiveModel::Conversion
     include Callbacks
@@ -39,7 +40,6 @@ class Base
     include Reflection
     include Serialization
 
-    include Indexing
     include AttachedFiles
     include FedoraAttributes
     include AttributeMethods

lib/active_fedora/indexing.rb

@@ -61,6 +61,35 @@ def update_index
       end
     end
 
+    protected
+
+      # Determines whether a create operation causes a solr index of this object by default.
+      # Override this if you need different behavior.
+      def create_needs_index?
+        ENABLE_SOLR_UPDATES
+      end
+
+      # Determines whether an update operation causes a solr index of this object by default.
+      # Override this if you need different behavior
+      def update_needs_index?
+        ENABLE_SOLR_UPDATES
+      end
+
+    private
+
+      # index the record after it has been persisted to Fedora
+      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 = {})
+        super
+        update_index if update_needs_index? && options.fetch(:update_index, true)
+        true
+      end
 
     module ClassMethods
 

lib/active_fedora/persistence.rb

@@ -40,11 +40,6 @@ def update(attributes)
 
     alias update_attributes update
 
-    def refresh
-      @ldp_source = LdpResource.new(conn, uri)
-      @resource = nil
-    end
-
     #Deletes a Base object, also deletes the info indexed in Solr, and
     #the underlying inner_object.  If this object is held in any relationships (ie inbound relationships
     #outside of this object it will remove it from those items rels-ext as well
@@ -136,20 +131,6 @@ def delete_tombstone uri
       end
     end
 
-  protected
-
-    # Determines whether a create operation causes a solr index of this object by default.
-    # Override this if you need different behavior.
-    def create_needs_index?
-      ENABLE_SOLR_UPDATES
-    end
-
-    # Determines whether an update operation causes a solr index of this object by default.
-    # Override this if you need different behavior
-    def update_needs_index?
-      ENABLE_SOLR_UPDATES
-    end
-
   private
 
     # Deals with preparing new object to be saved to Fedora, then pushes it and its attached files into Fedora.
@@ -159,17 +140,20 @@ def create_record(options = {})
       @ldp_source = @ldp_source.create
       @resource = nil
       assign_uri_to_attached_files
-      should_update_index = create_needs_index? && options.fetch(:update_index, true)
-      persist(should_update_index)
-      true
+      save_attached_files
+      refresh
     end
 
     def update_record(options = {})
       serialize_attached_files
       execute_sparql_update
-      should_update_index = update_needs_index? && options.fetch(:update_index, true)
-      persist(should_update_index)
-      true
+      save_attached_files
+      refresh
+    end
+
+    def refresh
+      @ldp_source = LdpResource.new(conn, uri)
+      @resource = nil
     end
 
     def execute_sparql_update
@@ -198,13 +182,10 @@ def assign_uri_to_attached_files
       end
     end
 
-    def persist(should_update_index)
+    def save_attached_files
       attached_files.select { |_, ds| ds.changed? }.each do |_, ds|
         ds.save # Don't call save! because if the content_changed? returns false, it'll raise an error.
       end
-
-      refresh
-      update_index if should_update_index
     end
   end
 end

spec/unit/indexing_spec.rb

@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe ActiveFedora::Indexing do
+  context "internal methods" do
+    before :all do
+      class SpecNode
+        include ActiveFedora::Indexing
+      end
+    end
+    after :all do
+      Object.send(:remove_const, :SpecNode)
+    end
+
+    subject { SpecNode.new }
+
+    describe "#create_needs_index?" do
+      subject { SpecNode.new.send(:create_needs_index?) }
+      it { should be true }
+    end
+
+    describe "#update_needs_index?" do
+      subject { SpecNode.new.send(:update_needs_index?) }
+      it { should be true }
+    end
+  end
+end

spec/unit/persistence_spec.rb

@@ -1,28 +1,6 @@
 require 'spec_helper'
 
 describe ActiveFedora::Persistence do
-  context "internal methods" do
-    before :all do
-      class SpecNode
-        include ActiveFedora::Persistence
-      end
-    end
-    after :all do
-      Object.send(:remove_const, :SpecNode)
-    end
-
-    subject { SpecNode.new }
-
-    describe "#create_needs_index?" do
-      subject { SpecNode.new.send(:create_needs_index?) }
-      it { should be true }
-    end
-
-    describe "#update_needs_index?" do
-      subject { SpecNode.new.send(:update_needs_index?) }
-      it { should be true }
-    end
-  end
 
   describe "an unsaved object" do
     subject { ActiveFedora::Base.new }
@@ -57,7 +35,7 @@ class SpecNode
     context "when called with option update_index: false" do
       context "on a new record" do
         it "should not update the index" do
-          expect(subject).to receive(:persist).with(false)
+          expect(subject).to_not receive(:update_index)
           subject.save(update_index: false)
         end
       end
@@ -70,7 +48,7 @@ class SpecNode
         end
 
         it "should not update the index" do
-          expect(subject).to receive(:persist).with(false)
+          expect(subject).to_not receive(:update_index)
           subject.save(update_index: false)
         end
       end
@@ -81,7 +59,7 @@ class SpecNode
         before { allow(subject).to receive(:create_needs_index?) { false } }
 
         it "should not override `create_needs_index?'" do
-          expect(subject).to receive(:persist).with(false)
+          expect(subject).to_not receive(:update_index)
           subject.save(update_index: true)
         end
       end
@@ -95,7 +73,7 @@ class SpecNode
         end
 
         it "should not override `update_needs_index?'" do
-          expect(subject).to receive(:persist).with(false)
+          expect(subject).to_not receive(:update_index)
           subject.save(update_index: true)
         end
       end