Fixes #3109 - Race condition between #lookup and .update causes caching of obsolete data.
This commit is contained in:
parent
2d861508f4
commit
d6f5baa87e
2 changed files with 34 additions and 9 deletions
|
@ -46,17 +46,42 @@ returns
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def find_and_save_to_cache_by(attr)
|
def find_and_save_to_cache_by(args)
|
||||||
record = find_by(attr)
|
attribute = args.keys.first
|
||||||
return nil if string_key?(attr.keys.first) && (record&.send(attr.keys.first) != attr.values.first.to_s) # enforce case-sensitivity on MySQL
|
lookup_value = args.values.first.to_s
|
||||||
return record if ActiveRecord::Base.connection.transaction_open? # rollbacks can invalidate cache entries
|
|
||||||
|
|
||||||
cache_set(attr.values.first, record)
|
# rollbacks can invalidate cache entry
|
||||||
record
|
# therefore we don't write it
|
||||||
|
if ActiveRecord::Base.connection.transaction_open?
|
||||||
|
result = find_by(attribute => lookup_value)
|
||||||
|
# enforce case-sensitivity on MySQL
|
||||||
|
result = nil if !key_sensitive_match?(result, attribute, lookup_value)
|
||||||
|
else
|
||||||
|
# get the record via an `FOR UPDATE` DB lock inside of
|
||||||
|
# a transaction to ensure that we don't write obsolete
|
||||||
|
# data into the cache
|
||||||
|
transaction do
|
||||||
|
result = lock.find_by(attribute => lookup_value)
|
||||||
|
# enforce case-sensitivity on MySQL
|
||||||
|
if key_sensitive_match?(result, attribute, lookup_value)
|
||||||
|
# cache only if we got a key-sensitive match
|
||||||
|
cache_set(lookup_value, result)
|
||||||
|
else
|
||||||
|
# no key-sensitive match - no result
|
||||||
|
result = nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
result
|
||||||
end
|
end
|
||||||
|
|
||||||
def string_key?(key)
|
def key_sensitive_match?(record, attribute, lookup_value)
|
||||||
type_for_attribute(key.to_s).type == :string
|
return false if record.blank?
|
||||||
|
return true if type_for_attribute(attribute.to_s).type != :string
|
||||||
|
|
||||||
|
record[attribute] == lookup_value
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1680,7 +1680,7 @@ result
|
||||||
def build_sms_recipients_list(value, article)
|
def build_sms_recipients_list(value, article)
|
||||||
Array(value['recipient'])
|
Array(value['recipient'])
|
||||||
.each_with_object([]) { |recipient_type, sum| sum.concat(Array(sms_recipients_by_type(recipient_type, article))) }
|
.each_with_object([]) { |recipient_type, sum| sum.concat(Array(sms_recipients_by_type(recipient_type, article))) }
|
||||||
.map { |user_or_id| User.lookup(id: user_or_id) }
|
.map { |user_or_id| user_or_id.is_a?(User) ? user_or_id : User.lookup(id: user_or_id) }
|
||||||
.uniq(&:id)
|
.uniq(&:id)
|
||||||
.select { |user| user.mobile.present? }
|
.select { |user| user.mobile.present? }
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue