Extract `Repository.memoize_method` method
And reuse `Gitlab::Utils::StrongMemoize`. There is a subtle behavior change required to reuse StrongMemoize in this case. The early fallback check now occurs *before* reading the memoized value instead of after. I think this is fine since a memoized value should only exist if `exists?` is also already memoized as `true`.
This commit is contained in:
Michael Kozono
parent
0920342094
commit
d9c4ebc5a0
|
|
@ -612,7 +612,7 @@ class Repository
|
|||
|
||||
Licensee::License.new(license_key)
|
||||
end
|
||||
cache_method :license, memoize_only: true
|
||||
memoize_method :license
|
||||
|
||||
def gitignore
|
||||
file_on_head(:gitignore)
|
||||
|
|
|
|||
|
|
@ -1,19 +1,41 @@
|
|||
module Gitlab
|
||||
module RepositoryCacheAdapter
|
||||
extend ActiveSupport::Concern
|
||||
include Gitlab::Utils::StrongMemoize
|
||||
|
||||
class_methods do
|
||||
# Wraps around the given method and caches its output in Redis and an instance
|
||||
# variable.
|
||||
# Caches and strongly memoizes the method.
|
||||
#
|
||||
# This only works for methods that do not take any arguments.
|
||||
def cache_method(name, fallback: nil, memoize_only: false)
|
||||
#
|
||||
# name - The name of the method to be cached.
|
||||
# fallback - A value to fall back to if the repository does not exist, or
|
||||
# in case of a Git error. Defaults to nil.
|
||||
def cache_method(name, fallback: nil)
|
||||
wrap_method(name, :cache_method_output, fallback: fallback)
|
||||
end
|
||||
|
||||
# Strongly memoizes the method.
|
||||
#
|
||||
# This only works for methods that do not take any arguments.
|
||||
#
|
||||
# name - The name of the method to be memoized.
|
||||
# fallback - A value to fall back to if the repository does not exist, or
|
||||
# in case of a Git error. Defaults to nil. The fallback value
|
||||
# is not memoized.
|
||||
def memoize_method(name, fallback: nil)
|
||||
wrap_method(name, :memoize_method_output, fallback: fallback)
|
||||
end
|
||||
|
||||
# Prepends "_uncached_" to the target method name, and redefines the method
|
||||
# but wraps it in the `wrapper` method.
|
||||
def wrap_method(name, wrapper, *options)
|
||||
original = :"_uncached_#{name}"
|
||||
|
||||
alias_method(original, name)
|
||||
|
||||
define_method(name) do
|
||||
cache_method_output(name, fallback: fallback, memoize_only: memoize_only) do
|
||||
__send__(wrapper, name, *options) do # rubocop:disable GitlabSecurity/PublicSend
|
||||
__send__(original) # rubocop:disable GitlabSecurity/PublicSend
|
||||
end
|
||||
end
|
||||
|
|
@ -30,65 +52,69 @@ module Gitlab
|
|||
raise NotImplementedError
|
||||
end
|
||||
|
||||
# Caches the supplied block both in a cache and in an instance variable.
|
||||
# Caches and strongly memoizes the supplied block.
|
||||
#
|
||||
# The cache key and instance variable are named the same way as the value of
|
||||
# the `key` argument.
|
||||
#
|
||||
# This method will return `nil` if the corresponding instance variable is also
|
||||
# set to `nil`. This ensures we don't keep yielding the block when it returns
|
||||
# `nil`.
|
||||
#
|
||||
# key - The name of the key to cache the data in.
|
||||
# fallback - A value to fall back to in the event of a Git error.
|
||||
def cache_method_output(key, fallback: nil, memoize_only: false, &block)
|
||||
ivar = cache_instance_variable_name(key)
|
||||
|
||||
if instance_variable_defined?(ivar)
|
||||
instance_variable_get(ivar)
|
||||
else
|
||||
# If the repository doesn't exist and a fallback was specified we return
|
||||
# that value inmediately. This saves us Rugged/gRPC invocations.
|
||||
return fallback unless fallback.nil? || cache.repository.exists?
|
||||
|
||||
begin
|
||||
value =
|
||||
if memoize_only
|
||||
yield
|
||||
else
|
||||
cache.fetch(key, &block)
|
||||
end
|
||||
|
||||
instance_variable_set(ivar, value)
|
||||
rescue Gitlab::Git::Repository::NoRepository
|
||||
# Even if the above `#exists?` check passes these errors might still
|
||||
# occur (for example because of a non-existing HEAD). We want to
|
||||
# gracefully handle this and not cache anything
|
||||
fallback
|
||||
end
|
||||
# name - The name of the method to be cached.
|
||||
# fallback - A value to fall back to if the repository does not exist, or
|
||||
# in case of a Git error. Defaults to nil.
|
||||
def cache_method_output(name, fallback: nil, &block)
|
||||
memoize_method_output(name, fallback: fallback) do
|
||||
cache.fetch(name, &block)
|
||||
end
|
||||
end
|
||||
|
||||
# Strongly memoizes the supplied block.
|
||||
#
|
||||
# name - The name of the method to be memoized.
|
||||
# fallback - A value to fall back to if the repository does not exist, or
|
||||
# in case of a Git error. Defaults to nil. The fallback value is
|
||||
# not memoized.
|
||||
def memoize_method_output(name, fallback: nil, &block)
|
||||
no_repository_fallback(name, fallback: fallback) do
|
||||
strong_memoize(memoizable_name(name), &block)
|
||||
end
|
||||
end
|
||||
|
||||
# Returns the fallback value if the repository does not exist
|
||||
def no_repository_fallback(name, fallback: nil, &block)
|
||||
# Avoid unnecessary gRPC invocations
|
||||
return fallback if fallback && fallback_early?(name)
|
||||
|
||||
yield
|
||||
rescue Gitlab::Git::Repository::NoRepository
|
||||
# Even if the `#exists?` check in `fallback_early?` passes, these errors
|
||||
# might still occur (for example because of a non-existing HEAD). We
|
||||
# want to gracefully handle this and not memoize anything.
|
||||
fallback
|
||||
end
|
||||
|
||||
# Expires the caches of a specific set of methods
|
||||
def expire_method_caches(methods)
|
||||
methods.each do |key|
|
||||
unless cached_methods.include?(key.to_sym)
|
||||
Rails.logger.error "Requested to expire non-existent method '#{key}' for Repository"
|
||||
methods.each do |name|
|
||||
unless cached_methods.include?(name.to_sym)
|
||||
Rails.logger.error "Requested to expire non-existent method '#{name}' for Repository"
|
||||
next
|
||||
end
|
||||
|
||||
cache.expire(key)
|
||||
cache.expire(name)
|
||||
|
||||
ivar = cache_instance_variable_name(key)
|
||||
|
||||
remove_instance_variable(ivar) if instance_variable_defined?(ivar)
|
||||
clear_memoization(memoizable_name(name))
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def cache_instance_variable_name(key)
|
||||
:"@#{key.to_s.tr('?!', '')}"
|
||||
def memoizable_name(name)
|
||||
"#{name.to_s.tr('?!', '')}"
|
||||
end
|
||||
|
||||
# All cached repository methods depend on the existence of a Git repository,
|
||||
# so if the repository doesn't exist, we already know not to call it.
|
||||
def fallback_early?(method_name)
|
||||
# Avoid infinite loop
|
||||
return false if method_name == :exists?
|
||||
|
||||
!exists?
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -65,6 +65,64 @@ describe Gitlab::RepositoryCacheAdapter do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#memoize_method_output' do
|
||||
let(:fallback) { 10 }
|
||||
|
||||
context 'with a non-existing repository' do
|
||||
let(:project) { create(:project) } # No repository
|
||||
|
||||
subject do
|
||||
repository.memoize_method_output(:cats, fallback: fallback) do
|
||||
repository.cats_call_stub
|
||||
end
|
||||
end
|
||||
|
||||
it 'returns the fallback value' do
|
||||
expect(subject).to eq(fallback)
|
||||
end
|
||||
|
||||
it 'avoids calling the original method' do
|
||||
expect(repository).not_to receive(:cats_call_stub)
|
||||
|
||||
subject
|
||||
end
|
||||
|
||||
it 'does not set the instance variable' do
|
||||
subject
|
||||
|
||||
expect(repository.instance_variable_defined?(:@cats)).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with a method throwing a non-existing-repository error' do
|
||||
subject do
|
||||
repository.memoize_method_output(:cats, fallback: fallback) do
|
||||
raise Gitlab::Git::Repository::NoRepository
|
||||
end
|
||||
end
|
||||
|
||||
it 'returns the fallback value' do
|
||||
expect(subject).to eq(fallback)
|
||||
end
|
||||
|
||||
it 'does not set the instance variable' do
|
||||
subject
|
||||
|
||||
expect(repository.instance_variable_defined?(:@cats)).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an existing repository' do
|
||||
it 'sets the instance variable' do
|
||||
repository.memoize_method_output(:cats, fallback: fallback) do
|
||||
'block output'
|
||||
end
|
||||
|
||||
expect(repository.instance_variable_get(:@cats)).to eq('block output')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#expire_method_caches' do
|
||||
it 'expires the caches of the given methods' do
|
||||
expect(cache).to receive(:expire).with(:rendered_readme)
|
||||
|
|
|
|||
Loading…
Reference in New Issue