Make swap impenetrable by concurrency

So, currently, these is still a teeny-tiny chance that on subsequent
attachment updates an older update overwrites a newer, if another UPDATE
happens between reloading and updating. I've tried preventing that with
a database transaction, but I apparently back then didn't understand
transactions good because I thought they act like mutexes.

The right way to make "check and update" atomic is simply making an
UPDATE...WHERE query. Then afterwards we reload the record so that it
has the actual attachment value, and then we just check whether the
attachment value is our stored file. If it's not, we return nil, so that
`Attacher#promote` knows that the swap failed and knows to delete the
unused stored file.

Hopefully this should now ensure that there are no concurrency issues.

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## HEAD
 
+* Remove a tiny possibility of a race condition with backgrounding on subsequent updates (janko-m)
+
 * Add `:delegate` option to migration_helpers for opting out of defining methods on the model (janko-m)
 
 * Make logging plugin log number of both input and output files for processing (janko-m)

lib/shrine.rb

@@ -590,7 +590,7 @@ def promote?(uploaded_file)
         # Calls #update, overriden in ORM plugins.
         def swap(uploaded_file)
           update(uploaded_file)
-          uploaded_file
+          uploaded_file if get == uploaded_file
         end
 
         # Sets and saves the uploaded file.

lib/shrine/plugins/activerecord.rb

@@ -75,18 +75,12 @@ module AttacherMethods
 
         # Updates the current attachment with the new one, unless the current
         # attachment has changed.
-        def swap(uploaded_file)
-          record.class.transaction do
-            break if record.send("#{name}_data") != record.reload.send("#{name}_data")
-            super
-          end
-        rescue ::ActiveRecord::RecordNotFound
-        end
-
-        # We save the record after updating, raising any validation errors.
         def update(uploaded_file)
-          super
-          record.save!
+          record.class.where(record.class.primary_key => record.id)
+            .where(:"#{name}_data" => record.send(:"#{name}_data"))
+            .update_all(:"#{name}_data" => uploaded_file.to_json)
+          record.reload
+        rescue ::ActiveRecord::RecordNotFound
         end
       end
     end

lib/shrine/plugins/sequel.rb

@@ -73,18 +73,12 @@ module AttacherMethods
 
         # Updates the current attachment with the new one, unless the current
         # attachment has changed.
-        def swap(uploaded_file)
-          record.db.transaction do
-            break if record.send("#{name}_data") != record.reload.send("#{name}_data")
-            super
-          end
-        rescue ::Sequel::Error
-        end
-
-        # We save the record after updating, raising any validation errors.
         def update(uploaded_file)
-          super
-          record.save(raise_on_failure: true)
+          record.this
+            .where(:"#{name}_data" => record.send(:"#{name}_data"))
+            .update(:"#{name}_data" => uploaded_file.to_json)
+          record.reload
+        rescue ::Sequel::Error
         end
 
         # Support for Postgres JSON columns.

test/attacher_test.rb

@@ -139,7 +139,7 @@
       io = fakeio
       context = {name: @attacher.name, record: @attacher.record, phase: :store}
       @attacher.assign(io)
-      @attacher.store.expects(:upload).with(@attacher.get, context).returns(true)
+      @attacher.store.expects(:upload).with(@attacher.get, context).returns(@attacher.get)
       @attacher._promote
     end