Refactoring: Deprecate custom Cache implementation and wrap native Rails cache instead.

This commit is contained in:
Mantas Masalskis 2021-05-31 13:05:54 +00:00 committed by Thorsten Eckel
parent 328845c283
commit c45b5ac51f
33 changed files with 67 additions and 193 deletions

View file

@ -48,7 +48,7 @@ class ChannelsTwitterController < ApplicationController
end end
def webhook_verify def webhook_verify
external_credential = Cache.get('external_credential_twitter') external_credential = Cache.read('external_credential_twitter')
if !external_credential && ExternalCredential.exists?(name: 'twitter') if !external_credential && ExternalCredential.exists?(name: 'twitter')
external_credential = ExternalCredential.find_by(name: 'twitter').credentials external_credential = ExternalCredential.find_by(name: 'twitter').credentials
end end

View file

@ -119,7 +119,7 @@ returns
def attributes_with_association_ids def attributes_with_association_ids
key = "#{self.class}::aws::#{id}" key = "#{self.class}::aws::#{id}"
cache = Cache.get(key) cache = Cache.read(key)
return cache if cache return cache if cache
attributes = self.attributes attributes = self.attributes

View file

@ -19,7 +19,7 @@ returns
def latest_change def latest_change
key = "#{name}_latest_change" key = "#{name}_latest_change"
updated_at = Cache.get(key) updated_at = Cache.read(key)
return updated_at if updated_at return updated_at if updated_at

View file

@ -47,7 +47,7 @@ module ApplicationModel::HasCache
def cache_get(data_id) def cache_get(data_id)
key = "#{self}::#{data_id}" key = "#{self}::#{data_id}"
Cache.get(key) Cache.read(key)
end end
end end
end end

View file

@ -32,7 +32,7 @@ returns calendar object
end end
# prevent multiple setups for same ip # prevent multiple setups for same ip
cache = Cache.get('Calendar.init_setup.done') cache = Cache.read('Calendar.init_setup.done')
return if cache && cache[:ip] == ip return if cache && cache[:ip] == ip
Cache.write('Calendar.init_setup.done', { ip: ip }, { expires_in: 1.hour }) Cache.write('Calendar.init_setup.done', { ip: ip }, { expires_in: 1.hour })
@ -156,7 +156,7 @@ returns
# only sync every 5 days # only sync every 5 days
if id if id
cache_key = "CalendarIcal::#{id}" cache_key = "CalendarIcal::#{id}"
cache = Cache.get(cache_key) cache = Cache.read(cache_key)
return if !last_log && cache && cache[:ical_url] == ical_url return if !last_log && cache && cache[:ical_url] == ical_url
end end

View file

@ -83,7 +83,7 @@ class Cti::Driver::Placetel < Cti::Driver::Base
def load_voip_users def load_voip_users
return {} if @config.blank? || @config[:api_token].blank? return {} if @config.blank? || @config[:api_token].blank?
list = Cache.get('placetelGetVoipUsers') list = Cache.read('placetelGetVoipUsers')
return list if list return list if list
response = UserAgent.post( response = UserAgent.post(

View file

@ -27,7 +27,7 @@ class Cti::Driver::SipgateIo < Cti::Driver::Base
def load_voip_users def load_voip_users
return {} if @config.blank? || @config[:api_user].blank? || @config[:api_password].blank? return {} if @config.blank? || @config[:api_user].blank? || @config[:api_password].blank?
list = Cache.get('sipgateUserList') list = Cache.read('sipgateUserList')
return list if list return list if list
url = 'https://api.sipgate.com/v2/users' url = 'https://api.sipgate.com/v2/users'

View file

@ -26,7 +26,7 @@ add karma activity log of an object
# to skip the time loss of the transaction # to skip the time loss of the transaction
# to increase performance # to increase performance
if !force if !force
cache = Cache.get("Karma::ActivityLog.add::#{activity.once_ttl.seconds}::#{action}::#{user.id}::#{object}::#{o_id}") cache = Cache.read("Karma::ActivityLog.add::#{activity.once_ttl.seconds}::#{action}::#{user.id}::#{object}::#{o_id}")
return cache if cache return cache if cache
end end

View file

@ -27,7 +27,7 @@ class KnowledgeBase::Answer < ApplicationModel
def attributes_with_association_ids def attributes_with_association_ids
key = "#{self.class}::aws::#{id}" key = "#{self.class}::aws::#{id}"
cache = Cache.get(key) cache = Cache.read(key)
return cache if cache return cache if cache
attrs = super attrs = super

View file

@ -28,7 +28,7 @@ class KnowledgeBase::Answer::Translation < ApplicationModel
def attributes_with_association_ids def attributes_with_association_ids
key = "#{self.class}::aws::#{id}" key = "#{self.class}::aws::#{id}"
cache = Cache.get(key) cache = Cache.read(key)
return cache if cache return cache if cache
attrs = super attrs = super

View file

@ -126,7 +126,7 @@ reload config settings
end end
end end
@@change_id = Cache.get('Setting::ChangeId') # rubocop:disable Style/ClassVars @@change_id = Cache.read('Setting::ChangeId') # rubocop:disable Style/ClassVars
@@lookup_at = Time.now.to_i # rubocop:disable Style/ClassVars @@lookup_at = Time.now.to_i # rubocop:disable Style/ClassVars
true true
end end
@ -163,7 +163,7 @@ reload config settings
return true return true
end end
change_id = Cache.get('Setting::ChangeId') change_id = Cache.read('Setting::ChangeId')
if @@change_id && change_id == @@change_id if @@change_id && change_id == @@change_id
@@lookup_at = Time.now.to_i # rubocop:disable Style/ClassVars @@lookup_at = Time.now.to_i # rubocop:disable Style/ClassVars
#logger.debug "Setting.cache_valid?: cache still valid, #{@@change_id}/#{change_id}" #logger.debug "Setting.cache_valid?: cache still valid, #{@@change_id}/#{change_id}"

View file

@ -294,7 +294,7 @@ returns
local_sha = Digest::SHA256.hexdigest(content) local_sha = Digest::SHA256.hexdigest(content)
cache_key = "image-resize-#{local_sha}_#{width}" cache_key = "image-resize-#{local_sha}_#{width}"
image = Cache.get(cache_key) image = Cache.read(cache_key)
return image if image return image if image
temp_file = ::Tempfile.new temp_file = ::Tempfile.new

View file

@ -357,7 +357,7 @@ class Transaction::Notification
end end
def possible_recipients_of_group(group_id) def possible_recipients_of_group(group_id)
cache = Cache.get("Transaction::Notification.group_access.full::#{group_id}") cache = Cache.read("Transaction::Notification.group_access.full::#{group_id}")
return cache if cache return cache if cache
possible_recipients = User.group_access(group_id, 'full').sort_by(&:login) possible_recipients = User.group_access(group_id, 'full').sort_by(&:login)

View file

@ -119,7 +119,7 @@ class Transaction::Slack
md5_webhook = Digest::MD5.hexdigest(local_config['webhook']) md5_webhook = Digest::MD5.hexdigest(local_config['webhook'])
cache_key = "slack::backend::#{@item[:type]}::#{ticket.id}::#{md5_webhook}" cache_key = "slack::backend::#{@item[:type]}::#{ticket.id}::#{md5_webhook}"
if sent_value if sent_value
value = Cache.get(cache_key) value = Cache.read(cache_key)
if value == sent_value if value == sent_value
Rails.logger.debug { "did not send webhook, already sent (#{@item[:type]}/#{ticket.id}/#{local_config['webhook']})" } Rails.logger.debug { "did not send webhook, already sent (#{@item[:type]}/#{ticket.id}/#{local_config['webhook']})" }
next next

View file

@ -502,7 +502,7 @@ Get source file at https://i18n.zammad.com/api/v1/translations_empty_translation
private_class_method :cache_set private_class_method :cache_set
def self.cache_get(locale) def self.cache_get(locale)
Cache.get("TranslationMapOnlyContent::#{locale.downcase}") Cache.read("TranslationMapOnlyContent::#{locale.downcase}")
end end
private_class_method :cache_get private_class_method :cache_get
end end

View file

@ -44,7 +44,7 @@ returns
# get linked accounts # get linked accounts
local_attributes['accounts'] = {} local_attributes['accounts'] = {}
key = "User::authorizations::#{id}" key = "User::authorizations::#{id}"
local_accounts = Cache.get(key) local_accounts = Cache.read(key)
if !local_accounts if !local_accounts
local_accounts = {} local_accounts = {}
authorizations = self.authorizations() authorizations = self.authorizations()

View file

@ -32,7 +32,7 @@ module Zammad
config.api_path = '/api/v1' config.api_path = '/api/v1'
# define cache store # define cache store
config.cache_store = :file_store, Rails.root.join('tmp', "cache_file_store_#{Rails.env}") config.cache_store = :zammad_file_store, Rails.root.join('tmp', "cache_file_store_#{Rails.env}"), { expires_in: 7.days }
# default preferences by permission # default preferences by permission
config.preferences_default_by_permission = { config.preferences_default_by_permission = {

13
lib/active_support/cache/store.rb vendored Normal file
View file

@ -0,0 +1,13 @@
# Cache.get => read alias for backwards compatibility
# Even if main codebase is migrated, custom addons may still use old syntax!
module ActiveSupport
module Cache
class Store
def get(key)
ActiveSupport::Deprecation.warn("Method 'get' is deprecated. Please use 'Cache.read' instead.")
read(key)
end
end
end
end

View file

@ -0,0 +1,14 @@
module ActiveSupport
module Cache
class ZammadFileStore < FileStore
def write(name, value, options = {})
# in certain cases, caches are deleted by other thread at same
# time, just log it
super
rescue => e
Rails.logger.error "Can't write cache #{key}: #{e.inspect}"
Rails.logger.error e
end
end
end
end

View file

@ -1,61 +1,4 @@
module Cache # load get => read alias
require 'active_support/cache/store'
=begin Cache = Rails.cache
delete a cache
Cache.delete('some_key')
=end
def self.delete(key)
Rails.cache.delete(key.to_s)
end
=begin
write a cache
Cache.write(
'some_key',
{ some: { data: { 'structure' } } },
{ expires_in: 24.hours, # optional, default 7 days }
)
=end
def self.write(key, data, params = {})
params[:expires_in] ||= 7.days
# in certain cases, caches are deleted by other thread at same
# time, just log it
Rails.cache.write(key.to_s, data, params)
rescue => e
Rails.logger.error "Can't write cache #{key}: #{e.inspect}"
Rails.logger.error e
end
=begin
get a cache
value = Cache.get('some_key')
=end
def self.get(key)
Rails.cache.read(key.to_s)
end
=begin
clear whole cache store
Cache.clear
=end
def self.clear
Rails.cache.clear
end
end

View file

@ -107,7 +107,7 @@ result
return if !item['from']['id'] return if !item['from']['id']
cache_key = "FB:User:Lookup:#{item['from']['id']}" cache_key = "FB:User:Lookup:#{item['from']['id']}"
cache = Cache.get(cache_key) cache = Cache.read(cache_key)
return cache if cache return cache if cache
begin begin

View file

@ -50,7 +50,7 @@ module Import
end end
def status_bg def status_bg
state = Cache.get('import:state') state = Cache.read('import:state')
return state if state return state if state
{ {

View file

@ -23,7 +23,7 @@ module Import
def statistic def statistic
# check cache # check cache
cache = Cache.get('import_otrs_stats') cache = Cache.read('import_otrs_stats')
return cache if cache return cache if cache
# retrieve statistic # retrieve statistic

View file

@ -33,7 +33,7 @@ returns
# this cache will optimize the preference catch performance # this cache will optimize the preference catch performance
# because of the yaml deserialization its pretty slow # because of the yaml deserialization its pretty slow
# on many tickets you we cache it. # on many tickets you we cache it.
user_preferences = Cache.get("NotificationFactory::Mailer.notification_settings::#{user.id}") user_preferences = Cache.read("NotificationFactory::Mailer.notification_settings::#{user.id}")
if user_preferences.blank? if user_preferences.blank?
user_preferences = user.preferences user_preferences = user.preferences

View file

@ -112,7 +112,7 @@ returns
def self.ticket_ids def self.ticket_ids
key = 'Report::TicketReopened::StateList' key = 'Report::TicketReopened::StateList'
ticket_state_ids = Cache.get(key) ticket_state_ids = Cache.read(key)
return ticket_state_ids if ticket_state_ids return ticket_state_ids if ticket_state_ids
ticket_state_types = Ticket::StateType.where(name: %w[closed merged removed]) ticket_state_types = Ticket::StateType.where(name: %w[closed merged removed])

View file

@ -22,7 +22,7 @@ class Sequencer
def total def total
if !dry_run if !dry_run
result = Cache.get(cache_key) result = Cache.read(cache_key)
end end
result ||= ldap_connection.count(ldap_config[:user_filter]) result ||= ldap_connection.count(ldap_config[:user_filter])

View file

@ -5,7 +5,7 @@ class Service::GeoCalendar::Zammad
# check cache # check cache
cache_key = "zammadgeocalendar::#{address}" cache_key = "zammadgeocalendar::#{address}"
cache = ::Cache.get(cache_key) cache = ::Cache.read(cache_key)
return cache if cache return cache if cache
# do lookup # do lookup

View file

@ -7,7 +7,7 @@ class Service::GeoIp::Zammad
# check cache # check cache
cache_key = "zammadgeoip::#{address}" cache_key = "zammadgeoip::#{address}"
cache = ::Cache.get(cache_key) cache = ::Cache.read(cache_key)
return cache if cache return cache if cache
# do lookup # do lookup

View file

@ -19,7 +19,7 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
def self.overview_history_append(overview, user_id) def self.overview_history_append(overview, user_id)
key = "TicketOverviewHistory::#{user_id}" key = "TicketOverviewHistory::#{user_id}"
history = Cache.get(key) || [] history = Cache.read(key) || []
history.prepend overview history.prepend overview
history.uniq! history.uniq!
@ -31,7 +31,7 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
end end
def self.overview_history_get(user_id) def self.overview_history_get(user_id)
Cache.get("TicketOverviewHistory::#{user_id}") Cache.read("TicketOverviewHistory::#{user_id}")
end end
def load def load
@ -93,7 +93,7 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
end end
def pull_overview? def pull_overview?
result = Cache.get("TicketOverviewPull::#{@user.id}") result = Cache.read("TicketOverviewPull::#{@user.id}")
Cache.delete("TicketOverviewPull::#{@user.id}") if result Cache.delete("TicketOverviewPull::#{@user.id}") if result
return true if result return true if result

View file

@ -4,106 +4,10 @@ RSpec.describe Cache do
describe '.get' do describe '.get' do
before { allow(Rails.cache).to receive(:read) } before { allow(Rails.cache).to receive(:read) }
it 'wraps Rails.cache.read' do it 'alias of Rails.cache.read' do
described_class.get('foo') described_class.read('foo')
expect(Rails.cache).to have_received(:read).with('foo')
end
context 'with a non-string argument' do
it 'passes a string' do
described_class.get(:foo)
expect(Rails.cache).to have_received(:read).with('foo') expect(Rails.cache).to have_received(:read).with('foo')
end end
end end
end end
describe '.write' do
it 'stores string values' do
expect { described_class.write('123', 'some value') }
.to change { described_class.get('123') }.to('some value')
end
it 'stores hash values' do
expect { described_class.write('123', { key: 'some value' }) }
.to change { described_class.get('123') }.to({ key: 'some value' })
end
it 'overwrites previous values' do
described_class.write('123', 'some value')
expect { described_class.write('123', { key: 'some value' }) }
.to change { described_class.get('123') }.to({ key: 'some value' })
end
it 'stores hash values with non-ASCII content' do
expect { described_class.write('123', { key: 'some valueöäüß' }) }
.to change { described_class.get('123') }.to({ key: 'some valueöäüß' })
end
context 'when expiring' do
# we need to travel to a fixed point in time
# to prevent influences of timezone / DST
before do
travel_to '1995-12-21 13:37 +0100'
end
it 'defaults to expires_in: 7.days' do
described_class.write('123', 'some value')
expect { travel 7.days - 1.second }.not_to change { described_class.get('123') }
expect { travel 2.seconds }.to change { described_class.get('123') }.to(nil)
end
it 'accepts a custom :expires_in option' do
described_class.write('123', 'some value', expires_in: 3.seconds)
expect { travel 4.seconds }.to change { described_class.get('123') }.to(nil)
end
end
end
describe '.delete' do
it 'deletes stored values' do
described_class.write('123', 'some value')
expect { described_class.delete('123') }
.to change { described_class.get('123') }.to(nil)
end
it 'is idempotent' do
described_class.write('123', 'some value')
described_class.delete('123')
expect { described_class.delete('123') }.not_to raise_error
end
end
describe '.clear' do
it 'deletes all stored values' do
described_class.write('123', 'some value')
described_class.write('456', 'some value')
expect { described_class.clear }
.to change { described_class.get('123') }.to(nil)
.and change { described_class.get('456') }.to(nil)
end
it 'is idempotent' do
described_class.write('123', 'some value')
described_class.clear
expect { described_class.clear }.not_to raise_error
end
context 'when cache directory is not present on disk' do
before { FileUtils.rm_rf(Rails.cache.cache_path) }
it 'does not raise an error' do
expect { described_class.clear }.not_to raise_error
end
end
end
end

View file

@ -94,10 +94,10 @@ RSpec.describe Calendar, type: :model do
context 'when called explicitly after creation' do context 'when called explicitly after creation' do
it 'writes #public_holidays to the cache (valid for 1 day)' do it 'writes #public_holidays to the cache (valid for 1 day)' do
expect(Cache.get("CalendarIcal::#{calendar.id}")).to be(nil) expect(Cache.read("CalendarIcal::#{calendar.id}")).to be(nil)
expect { calendar.sync } expect { calendar.sync }
.to change { Cache.get("CalendarIcal::#{calendar.id}") } .to change { Cache.read("CalendarIcal::#{calendar.id}") }
.to(calendar.attributes.slice('public_holidays', 'ical_url').symbolize_keys) .to(calendar.attributes.slice('public_holidays', 'ical_url').symbolize_keys)
end end

View file

@ -27,7 +27,7 @@ RSpec.describe Setting, type: :model do
it 'resets the cache key' do it 'resets the cache key' do
expect { described_class.set(setting.name, 'baz') } expect { described_class.set(setting.name, 'baz') }
.to change { Cache.get('foo') }.to(nil) .to change { Cache.read('foo') }.to(nil)
end end
end end
end end

View file

@ -590,7 +590,7 @@ RSpec.describe 'Integration Placetel', type: :request do
expect(log.duration_talking_time).to be_nil expect(log.duration_talking_time).to be_nil
# check if cache is filled # check if cache is filled
expect(Cache.get('placetelGetVoipUsers')['777008478072@example.com']).to eq('Bob Smith') expect(Cache.read('placetelGetVoipUsers')['777008478072@example.com']).to eq('Bob Smith')
params = 'event=IncomingCall&direction=in&to=030600000000&from=012345&call_id=1234567890-3&peer=777008478072@example.com' params = 'event=IncomingCall&direction=in&to=030600000000&from=012345&call_id=1234567890-3&peer=777008478072@example.com'
post "/api/v1/placetel/#{token}", params: params post "/api/v1/placetel/#{token}", params: params
@ -612,7 +612,7 @@ RSpec.describe 'Integration Placetel', type: :request do
expect(log.duration_talking_time).to be_nil expect(log.duration_talking_time).to be_nil
# check if cache is filled # check if cache is filled
expect(Cache.get('placetelGetVoipUsers')['777008478072@example.com']).to eq('Bob Smith') expect(Cache.read('placetelGetVoipUsers')['777008478072@example.com']).to eq('Bob Smith')
end end
end end
end end