diff --git a/app/models/application_model/can_lookup.rb b/app/models/application_model/can_lookup.rb index b703dc87f..ac16a216b 100644 --- a/app/models/application_model/can_lookup.rb +++ b/app/models/application_model/can_lookup.rb @@ -46,41 +46,17 @@ returns private - def find_and_save_to_cache_by(args) - attribute = args.keys.first - lookup_value = args.values.first.to_s + def find_and_save_to_cache_by(attr) + record = find_by(attr) + return nil if string_key?(attr.keys.first) && (record&.send(attr.keys.first) != attr.values.first.to_s) # enforce case-sensitivity on MySQL + return record if ActiveRecord::Base.connection.transaction_open? # rollbacks can invalidate cache entries - # rollbacks can invalidate cache entry - # 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 + cache_set(attr.values.first, record) + record end - def key_sensitive_match?(record, attribute, lookup_value) - return false if record.blank? - return true if type_for_attribute(attribute.to_s).type != :string - - record[attribute] == lookup_value + def string_key?(key) + type_for_attribute(key.to_s).type == :string end end diff --git a/app/models/application_model/has_cache.rb b/app/models/application_model/has_cache.rb index 6d2b444f6..de9ba6ce6 100644 --- a/app/models/application_model/has_cache.rb +++ b/app/models/application_model/has_cache.rb @@ -41,7 +41,8 @@ module ApplicationModel::HasCache def cache_set(data_id, data) key = "#{self}::#{data_id}" - Cache.write(key, data) + # cache for 4 hours max to lower impact of race conditions + Cache.write(key, data, expires_in: 4.hours) end def cache_get(data_id) diff --git a/spec/models/application_model/can_lookup_examples.rb b/spec/models/application_model/can_lookup_examples.rb index 47544887f..3fb99a17b 100644 --- a/spec/models/application_model/can_lookup_examples.rb +++ b/spec/models/application_model/can_lookup_examples.rb @@ -55,7 +55,7 @@ RSpec.shared_examples 'ApplicationModel::CanLookup' do it 'saves the value to the cache' do expect(Rails.cache) .to receive(:write) - .with("#{described_class}::#{instance.send(attribute)}", instance, { expires_in: 7.days }) + .with("#{described_class}::#{instance.send(attribute)}", instance, { expires_in: 4.hours }) .and_call_original expect { described_class.lookup(attribute => instance.send(attribute)) }