aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record/locking
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2015-01-09 09:49:21 -0700
committerSean Griffin <sean@thoughtbot.com>2015-01-09 09:56:15 -0700
commit13772bfa49325a8dc26410d2dcb555665a320f92 (patch)
tree623890d306f92c8139ef507606611a0546eddcdf /activerecord/lib/active_record/locking
parentc425d664247aa18239dae4c377dba5d397070249 (diff)
downloadrails-13772bfa49325a8dc26410d2dcb555665a320f92.tar.gz
rails-13772bfa49325a8dc26410d2dcb555665a320f92.tar.bz2
rails-13772bfa49325a8dc26410d2dcb555665a320f92.zip
Properly persist `lock_version` as 0 if the DB has no default
The reason this bug occured is that we never actually check to see if this column has changed from it's default, since it was never assigned and is not mutable. It appears I was wrong in b301c40224c6d15b539dbcc7485adb44d810f88c, with my statement of "there is no longer a case where a given value would differ from the default, but would not already be marked as changed." However, I chose not to revert the deletion of `initialize_internals_callback` from that commit, as I think a solution closer to where the problem lies is less likely to get erroneously removed. I'm not super happy with this solution, but it mirrors what is being done in `_update_record`, and a fix for one should work for the other. I toyed with the idea of changing the definition of `changed?` on the type to `changed_in_place?`. If we type cast the raw value, it'll break a test about updating not modifying the lock column if nothing else was changed. We could have the definition check if `raw_old_value` is `nil`, but this feels fragile and less intention revealing. It would, however, have the benefit of cleaning up old data that incorrectly persisted as `nil`. Fixes #18422
Diffstat (limited to 'activerecord/lib/active_record/locking')
-rw-r--r--activerecord/lib/active_record/locking/optimistic.rb14
1 files changed, 9 insertions, 5 deletions
diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index 9f053453bd..aeb1a4ddc6 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -66,6 +66,15 @@ module ActiveRecord
send(lock_col + '=', previous_lock_value + 1)
end
+ def _create_record(attribute_names = self.attribute_names, *) # :nodoc:
+ if locking_enabled?
+ # We always want to persist the locking version, even if we don't detect
+ # a change from the default, since the database might have no default
+ attribute_names |= [self.class.locking_column]
+ end
+ super
+ end
+
def _update_record(attribute_names = self.attribute_names) #:nodoc:
return super unless locking_enabled?
return 0 if attribute_names.empty?
@@ -185,11 +194,6 @@ module ActiveRecord
super.to_i
end
- def changed?(old_value, *)
- # Ensure we save if the default was `nil`
- super || old_value == 0
- end
-
def init_with(coder)
__setobj__(coder['subtype'])
end