diff --git a/app/models/application_model/can_lookup.rb b/app/models/application_model/can_lookup.rb index e5c41219e..c5e955928 100644 --- a/app/models/application_model/can_lookup.rb +++ b/app/models/application_model/can_lookup.rb @@ -48,7 +48,7 @@ returns 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) # enforce case-sensitivity on MySQL + 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 cache_set(attr.values.first, record) diff --git a/spec/models/application_model/can_lookup_examples.rb b/spec/models/application_model/can_lookup_examples.rb index 6b6a433f5..47544887f 100644 --- a/spec/models/application_model/can_lookup_examples.rb +++ b/spec/models/application_model/can_lookup_examples.rb @@ -64,6 +64,13 @@ RSpec.shared_examples 'ApplicationModel::CanLookup' do end end + if described_class.type_for_attribute(attribute).type == :string + # https://github.com/zammad/zammad/issues/3121 + it 'retrieves results from cache with value as symbol' do + expect(described_class.lookup(attribute => instance.send(attribute).to_sym)).to be_present + end + end + context 'when called a second time' do before { described_class.lookup(attribute => instance.send(attribute)) } diff --git a/spec/models/history_spec.rb b/spec/models/history_spec.rb index 03a892434..be84de9d2 100644 --- a/spec/models/history_spec.rb +++ b/spec/models/history_spec.rb @@ -189,4 +189,58 @@ RSpec.describe History, type: :model do end end end + + shared_examples 'lookup and create if needed' do |prefix| + let(:prefix) { prefix } + let(:value_string) { Faker::Lorem.word } + let(:value_symbol) { value_string.to_sym } + let(:method_name) { "#{prefix}_lookup" } + let(:cache_key) { "#{described_class}::#{prefix.capitalize}::#{value_string}" } + + context 'when object does not exist' do + it 'creates with a given String' do + expect(described_class.send(method_name, value_string)).to be_present + end + + it 'creates with a given Symbol' do + expect(described_class.send(method_name, value_symbol)).to be_present + end + end + + context 'when object exists' do + before do + described_class.send(method_name, value_string) + end + + it 'retrieves object with a given String' do + expect(described_class.send(method_name, value_string)).to be_present + end + + it 'hits cache with a given String' do + allow(Rails.cache).to receive(:read) + described_class.send(method_name, value_string) + expect(Rails.cache).to have_received(:read).with(cache_key) + end + + it 'retrieves object with a given Symbol' do + expect(described_class.send(method_name, value_symbol)).to be_present + end + + it 'hits cache with a given Symbol' do + allow(Rails.cache).to receive(:read) + described_class.send(method_name, value_symbol) + expect(Rails.cache).to have_received(:read).with(cache_key) + end + end + end + + # https://github.com/zammad/zammad/issues/3121 + describe '.type_lookup' do + include_examples 'lookup and create if needed', 'type' + end + + # https://github.com/zammad/zammad/issues/3121 + describe '.object_lookup' do + include_examples 'lookup and create if needed', 'object' + end end