clear up duplication between Persistence#destroy and Locking#destroy

2 files changed, 34 insertions, 27 deletions

diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index 64433f580a..a3412582fa 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -101,26 +101,29 @@ module ActiveRecord
           end
         end
 
-        def destroy #:nodoc:
-          return super unless locking_enabled?
+        def destroy_row
+          affected_rows = super
+
+          if locking_enabled? && affected_rows != 1
+            raise ActiveRecord::StaleObjectError.new(self, "destroy")
+          end
 
-          destroy_associations
+          affected_rows
+        end
 
-          if persisted?
-            table = self.class.arel_table
-            lock_col = self.class.locking_column
-            predicate = table[self.class.primary_key].eq(id).
-              and(table[lock_col].eq(send(lock_col).to_i))
+        def relation_for_destroy
+          relation = super
 
-            affected_rows = self.class.unscoped.where(predicate).delete_all
+          if locking_enabled?
+            column_name = self.class.locking_column
+            column      = self.class.columns_hash[column_name]
+            substitute  = connection.substitute_at(column, relation.bind_values.length)
 
-            unless affected_rows == 1
-              raise ActiveRecord::StaleObjectError.new(self, "destroy")
-            end
+            relation = relation.where(self.class.arel_table[column_name].eq(substitute))
+            relation.bind_values << [column, self[column_name].to_i]
           end
 
-          @destroyed = true
-          freeze
+          relation
         end
 
       module ClassMethods
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 35c922e979..bb504ae90f 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -124,19 +124,7 @@ module ActiveRecord
     # that no changes should be made (since they can't be persisted).
     def destroy
       destroy_associations
-
-      if persisted?
-        pk         = self.class.primary_key
-        column     = self.class.columns_hash[pk]
-        substitute = connection.substitute_at(column, 0)
-
-        relation = self.class.unscoped.where(
-          self.class.arel_table[pk].eq(substitute))
-
-        relation.bind_values = [[column, id]]
-        relation.delete_all
-      end
-
+      destroy_row if persisted?
       @destroyed = true
       freeze
     end
@@ -335,6 +323,22 @@ module ActiveRecord
     def destroy_associations
     end
 
+    def destroy_row
+      relation_for_destroy.delete_all
+    end
+
+    def relation_for_destroy
+      pk         = self.class.primary_key
+      column     = self.class.columns_hash[pk]
+      substitute = connection.substitute_at(column, 0)
+
+      relation = self.class.unscoped.where(
+        self.class.arel_table[pk].eq(substitute))
+
+      relation.bind_values = [[column, id]]
+      relation
+    end
+
     def create_or_update
       raise ReadOnlyRecord if readonly?
       result = new_record? ? create : update