Reduce number of transactions for #mark_collection_as_read

lib/unread/read_mark.rb

@@ -1,5 +1,5 @@
 class ReadMark < ActiveRecord::Base
-  belongs_to :readable, polymorphic: true
+  belongs_to :readable, polymorphic: true, inverse_of: :read_marks
 
   validates_presence_of :reader_id, :reader_type, :readable_type
 

lib/unread/readable.rb

@@ -27,23 +27,40 @@ def mark_collection_as_read(collection, reader)
             if global_timestamp && global_timestamp >= timestamp
               # The object is implicitly marked as read, so there is nothing to do
             else
-              # This transaction is needed, so that parent transaction won't rollback even there's an error.
-              ReadMark.transaction(requires_new: true) do
-                begin
-                  rm = obj.read_marks.where(reader_id: reader.id, reader_type: reader.class.base_class.name).first || obj.read_marks.build
-                  rm.reader_id   = reader.id
-                  rm.reader_type = reader.class.base_class.name
-                  rm.timestamp   = timestamp
-                  rm.save!
-                rescue ActiveRecord::RecordNotUnique
-                  raise ActiveRecord::Rollback
-                end
-              end
+              mark_collection_item_as_read(obj, reader, timestamp)
             end
           end
         end
       end
 
+      def mark_collection_item_as_read(obj, reader, timestamp)
+        marking_proc = proc {
+          rm = obj.read_marks.find_or_initialize_by(reader: reader)
+          rm.timestamp = timestamp
+          rm.save!
+        }
+
+        if using_postgresql?
+          # With PostgreSQL, a transaction is unusable after a unique constraint vialoation.
+          # To avoid this, nested transactions are required.
+          # http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Exception+handling+and+rolling+back
+          ReadMark.transaction(requires_new: true) do
+            begin
+              marking_proc.call
+            rescue ActiveRecord::RecordNotUnique
+              # The object is explicitly marked as read, so rollback the inner transaction
+              raise ActiveRecord::Rollback
+            end
+          end
+        else
+          begin
+            marking_proc.call
+          rescue ActiveRecord::RecordNotUnique
+            # The object is explicitly marked as read, so there is nothing to do
+          end
+        end
+      end
+
       # A scope with all items accessable for the given reader
       # It's used in cleanup_read_marks! to support a filtered cleanup
       # Should be overriden if a reader doesn't have access to all items

spec/unread/readable_spec.rb

@@ -269,8 +269,8 @@
     it "should mark the rest as read when the first record is not unique" do
       Email.mark_as_read! [ @email1 ], for: @reader
 
-      allow(@email1).to receive_message_chain("read_marks.build").and_return(@email1.read_marks.build)
-      allow(@email1).to receive_message_chain("read_marks.where").and_return([])
+      allow(@email1).to receive_message_chain("read_marks.find_or_initialize_by")
+        .and_return(@email1.read_marks.build(reader: @reader))
 
       expect do
         Email.mark_as_read! [ @email1, @email2 ], for: @reader
@@ -295,7 +295,6 @@
       expect(@email2.unread?(@reader)).to be_falsey
     end
 
-
     it "should mark all objects as read" do
       Email.mark_as_read! :all, for: @reader
 
@@ -330,6 +329,11 @@
         Email.mark_as_read! :foo, :bar
       }.to raise_error(ArgumentError)
     end
+
+    it "should work with STI readers" do
+      Email.mark_as_read! [ @email1 ], for: Customer.find(@sti_reader.id)
+      expect(@email1.unread?(@sti_reader)).to be_falsey
+    end
   end
 
   describe :cleanup_read_marks! do