From 145832d6c6637094d40e78b80f1c72c1029515fd Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Fri, 15 Jun 2018 22:53:39 +1100 Subject: [Rails5] Fix optimistic lock value Update the monkey-patch to make it work in Rails 5. --- ...lackst0ne-rails5-fix-optimistic-lock-values.yml | 5 + config/initializers/active_record_locking.rb | 111 +++++++++++---------- 2 files changed, 64 insertions(+), 52 deletions(-) create mode 100644 changelogs/unreleased/blackst0ne-rails5-fix-optimistic-lock-values.yml diff --git a/changelogs/unreleased/blackst0ne-rails5-fix-optimistic-lock-values.yml b/changelogs/unreleased/blackst0ne-rails5-fix-optimistic-lock-values.yml new file mode 100644 index 00000000000..1915dff73ab --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-fix-optimistic-lock-values.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Fix optimistic lock value" +merge_request: 19878 +author: "@blackst0ne" +type: fixed diff --git a/config/initializers/active_record_locking.rb b/config/initializers/active_record_locking.rb index 3e7111fd063..0861544c5a4 100644 --- a/config/initializers/active_record_locking.rb +++ b/config/initializers/active_record_locking.rb @@ -1,73 +1,80 @@ # rubocop:disable Lint/RescueException -# Remove this entire initializer when we are at rails 5.0. -# This file fixes the bug (see below) which has been fixed in the upstream. -unless Gitlab.rails5? - # This patch fixes https://github.com/rails/rails/issues/26024 - # TODO: Remove it when it's no longer necessary - - module ActiveRecord - module Locking - module Optimistic - # We overwrite this method because we don't want to have default value - # for newly created records - def _create_record(attribute_names = self.attribute_names, *) # :nodoc: - super - end +# Remove this monkey-patch when all lock_version values are converted from NULLs to zeros. +# See https://gitlab.com/gitlab-org/gitlab-ce/issues/25228 +module ActiveRecord + module Locking + module Optimistic + # We overwrite this method because we don't want to have default value + # for newly created records + def _create_record(attribute_names = self.attribute_names, *) # :nodoc: + super + end - def _update_record(attribute_names = self.attribute_names) #:nodoc: - return super unless locking_enabled? - return 0 if attribute_names.empty? + def _update_record(attribute_names = self.attribute_names) #:nodoc: + return super unless locking_enabled? + return 0 if attribute_names.empty? - lock_col = self.class.locking_column + lock_col = self.class.locking_column - previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend + previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend - # This line is added as a patch - previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0 + # This line is added as a patch + previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0 - increment_lock + increment_lock - attribute_names += [lock_col] - attribute_names.uniq! + attribute_names += [lock_col] + attribute_names.uniq! - begin - relation = self.class.unscoped + begin + relation = self.class.unscoped - affected_rows = relation.where( - self.class.primary_key => id, - lock_col => previous_lock_value - ).update_all( - attributes_for_update(attribute_names).map do |name| - [name, _read_attribute(name)] - end.to_h - ) + affected_rows = relation.where( + self.class.primary_key => id, + lock_col => previous_lock_value + ).update_all( + attributes_for_update(attribute_names).map do |name| + [name, _read_attribute(name)] + end.to_h + ) - unless affected_rows == 1 - raise ActiveRecord::StaleObjectError.new(self, "update") - end + unless affected_rows == 1 + raise ActiveRecord::StaleObjectError.new(self, "update") + end - affected_rows + affected_rows - # If something went wrong, revert the version. - rescue Exception - send(lock_col + '=', previous_lock_value) # rubocop:disable GitlabSecurity/PublicSend - raise - end + # If something went wrong, revert the version. + rescue Exception + send(lock_col + '=', previous_lock_value) # rubocop:disable GitlabSecurity/PublicSend + raise end + end - # This is patched because we need it to query `lock_version IS NULL` - # rather than `lock_version = 0` whenever lock_version is NULL. - def relation_for_destroy - return super unless locking_enabled? + # This is patched because we need it to query `lock_version IS NULL` + # rather than `lock_version = 0` whenever lock_version is NULL. + def relation_for_destroy + return super unless locking_enabled? - column_name = self.class.locking_column - super.where(self.class.arel_table[column_name].eq(self[column_name])) - end + column_name = self.class.locking_column + super.where(self.class.arel_table[column_name].eq(self[column_name])) end + end + + # This is patched because we want `lock_version` default to `NULL` + # rather than `0` + if Gitlab.rails5? + class LockingType + def deserialize(value) + super + end - # This is patched because we want `lock_version` default to `NULL` - # rather than `0` + def serialize(value) + super + end + end + else class LockingType < SimpleDelegator def type_cast_from_database(value) super -- cgit v1.2.1