Commit 5171e2f3 authored by Alejandro Rodríguez's avatar Alejandro Rodríguez

Refactor RepositoryCache to make it usable in other classes

parent 35f6efae
...@@ -16,6 +16,7 @@ class Repository ...@@ -16,6 +16,7 @@ class Repository
].freeze ].freeze
include Gitlab::ShellAdapter include Gitlab::ShellAdapter
include Gitlab::RepositoryCacheAdapter
attr_accessor :full_path, :disk_path, :project, :is_wiki attr_accessor :full_path, :disk_path, :project, :is_wiki
...@@ -57,22 +58,6 @@ class Repository ...@@ -57,22 +58,6 @@ class Repository
merge_request_template: :merge_request_template_names merge_request_template: :merge_request_template_names
}.freeze }.freeze
# Wraps around the given method and caches its output in Redis and an instance
# variable.
#
# This only works for methods that do not take any arguments.
def self.cache_method(name, fallback: nil, memoize_only: false)
original = :"_uncached_#{name}"
alias_method(original, name)
define_method(name) do
cache_method_output(name, fallback: fallback, memoize_only: memoize_only) do
__send__(original) # rubocop:disable GitlabSecurity/PublicSend
end
end
end
def initialize(full_path, project, disk_path: nil, is_wiki: false) def initialize(full_path, project, disk_path: nil, is_wiki: false)
@full_path = full_path @full_path = full_path
@disk_path = disk_path || full_path @disk_path = disk_path || full_path
...@@ -302,17 +287,6 @@ class Repository ...@@ -302,17 +287,6 @@ class Repository
expire_method_caches(CACHED_METHODS) expire_method_caches(CACHED_METHODS)
end end
# Expires the caches of a specific set of methods
def expire_method_caches(methods)
methods.each do |key|
cache.expire(key)
ivar = cache_instance_variable_name(key)
remove_instance_variable(ivar) if instance_variable_defined?(ivar)
end
end
def expire_avatar_cache def expire_avatar_cache
expire_method_caches(%i(avatar)) expire_method_caches(%i(avatar))
end end
...@@ -921,49 +895,6 @@ class Repository ...@@ -921,49 +895,6 @@ class Repository
end end
end end
# Caches the supplied block both in a cache and in an instance variable.
#
# 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? || 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
end
end
def cache_instance_variable_name(key)
:"@#{key.to_s.tr('?!', '')}"
end
def file_on_head(type) def file_on_head(type)
if head = tree(:head) if head = tree(:head)
head.blobs.find do |blob| head.blobs.find do |blob|
...@@ -1018,8 +949,7 @@ class Repository ...@@ -1018,8 +949,7 @@ class Repository
end end
def cache def cache
# TODO: should we use UUIDs here? We could move repositories without clearing this cache @cache ||= Gitlab::RepositoryCache.new(self)
@cache ||= RepositoryCache.new(full_path, @project.id)
end end
def tags_sorted_by_committed_date def tags_sorted_by_committed_date
......
# Interface to the Redis-backed cache store
module Gitlab
class RepositoryCache
attr_reader :repository, :namespace, :backend
def initialize(repository, backend = Rails.cache)
@repository = repository
@namespace = "#{repository.full_path}:#{repository.project.id}"
@backend = backend
end
def cache_key(type)
"#{type}:#{namespace}"
end
def expire(key)
backend.delete(cache_key(key))
end
def fetch(key, &block)
backend.fetch(cache_key(key), &block)
end
def exist?(key)
backend.exist?(cache_key(key))
end
def read(key)
backend.read(cache_key(key))
end
end
end
module Gitlab
module RepositoryCacheAdapter
extend ActiveSupport::Concern
class_methods do
# Wraps around the given method and caches its output in Redis and an instance
# variable.
#
# This only works for methods that do not take any arguments.
def cache_method(name, fallback: nil, memoize_only: false)
original = :"_uncached_#{name}"
alias_method(original, name)
define_method(name) do
cache_method_output(name, fallback: fallback, memoize_only: memoize_only) do
__send__(original) # rubocop:disable GitlabSecurity/PublicSend
end
end
end
end
# RepositoryCache to be used. Should be overridden by the including class
def cache
raise NotImplementedError
end
# Caches the supplied block both in a cache and in an instance variable.
#
# 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
end
end
# Expires the caches of a specific set of methods
def expire_method_caches(methods)
methods.each do |key|
cache.expire(key)
ivar = cache_instance_variable_name(key)
remove_instance_variable(ivar) if instance_variable_defined?(ivar)
end
end
private
def cache_instance_variable_name(key)
:"@#{key.to_s.tr('?!', '')}"
end
end
end
# Interface to the Redis-backed cache store used by the Repository model
class RepositoryCache
attr_reader :namespace, :backend, :project_id
def initialize(namespace, project_id, backend = Rails.cache)
@namespace = namespace
@backend = backend
@project_id = project_id
end
def cache_key(type)
"#{type}:#{namespace}:#{project_id}"
end
def expire(key)
backend.delete(cache_key(key))
end
def fetch(key, &block)
backend.fetch(cache_key(key), &block)
end
def exist?(key)
backend.exist?(cache_key(key))
end
def read(key)
backend.read(cache_key(key))
end
end
require 'spec_helper'
describe Gitlab::RepositoryCacheAdapter do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
let(:cache) { repository.send(:cache) }
describe '#cache_method_output', :use_clean_rails_memory_store_caching do
let(:fallback) { 10 }
context 'with a non-existing repository' do
let(:project) { create(:project) } # No repository
subject do
repository.cache_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
end
context 'with a method throwing a non-existing-repository error' do
subject do
repository.cache_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 cache the data' do
subject
expect(repository.instance_variable_defined?(:@cats)).to eq(false)
expect(cache.exist?(:cats)).to eq(false)
end
end
context 'with an existing repository' do
it 'caches the output' do
object = double
expect(object).to receive(:number).once.and_return(10)
2.times do
val = repository.cache_method_output(:cats) { object.number }
expect(val).to eq(10)
end
expect(repository.send(:cache).exist?(:cats)).to eq(true)
expect(repository.instance_variable_get(:@cats)).to eq(10)
end
end
end
describe '#expire_method_caches' do
it 'expires the caches of the given methods' do
expect(cache).to receive(:expire).with(:readme)
expect(cache).to receive(:expire).with(:gitignore)
repository.expire_method_caches(%i(readme gitignore))
end
end
end
require 'spec_helper' require 'spec_helper'
describe RepositoryCache do describe Gitlab::RepositoryCache do
let(:project) { create(:project) }
let(:backend) { double('backend').as_null_object } let(:backend) { double('backend').as_null_object }
let(:cache) { described_class.new('example', project.id, backend) } let(:project) { create(:project) }
let(:repository) { project.repository }
let(:namespace) { "#{repository.full_path}:#{project.id}" }
let(:cache) { described_class.new(repository, backend) }
describe '#cache_key' do describe '#cache_key' do
it 'includes the namespace' do it 'includes the namespace' do
expect(cache.cache_key(:foo)).to eq "foo:example:#{project.id}" expect(cache.cache_key(:foo)).to eq "foo:#{namespace}"
end end
end end
describe '#expire' do describe '#expire' do
it 'expires the given key from the cache' do it 'expires the given key from the cache' do
cache.expire(:foo) cache.expire(:foo)
expect(backend).to have_received(:delete).with("foo:example:#{project.id}") expect(backend).to have_received(:delete).with("foo:#{namespace}")
end end
end end
describe '#fetch' do describe '#fetch' do
it 'fetches the given key from the cache' do it 'fetches the given key from the cache' do
cache.fetch(:bar) cache.fetch(:bar)
expect(backend).to have_received(:fetch).with("bar:example:#{project.id}") expect(backend).to have_received(:fetch).with("bar:#{namespace}")
end end
it 'accepts a block' do it 'accepts a block' do
p = -> {} p = -> {}
cache.fetch(:baz, &p) cache.fetch(:baz, &p)
expect(backend).to have_received(:fetch).with("baz:example:#{project.id}", &p) expect(backend).to have_received(:fetch).with("baz:#{namespace}", &p)
end end
end end
end end
...@@ -2171,15 +2171,6 @@ describe Repository do ...@@ -2171,15 +2171,6 @@ describe Repository do
end end
end end
describe '#expire_method_caches' do
it 'expires the caches of the given methods' do
expect_any_instance_of(RepositoryCache).to receive(:expire).with(:readme)
expect_any_instance_of(RepositoryCache).to receive(:expire).with(:gitignore)
repository.expire_method_caches(%i(readme gitignore))
end
end
describe '#expire_all_method_caches' do describe '#expire_all_method_caches' do
it 'expires the caches of all methods' do it 'expires the caches of all methods' do
expect(repository).to receive(:expire_method_caches) expect(repository).to receive(:expire_method_caches)
...@@ -2325,66 +2316,6 @@ describe Repository do ...@@ -2325,66 +2316,6 @@ describe Repository do
end end
end end
describe '#cache_method_output', :use_clean_rails_memory_store_caching do
let(:fallback) { 10 }
context 'with a non-existing repository' do
let(:project) { create(:project) } # No repository
subject do
repository.cache_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
end
context 'with a method throwing a non-existing-repository error' do
subject do
repository.cache_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 cache the data' do
subject
expect(repository.instance_variable_defined?(:@cats)).to eq(false)
expect(repository.send(:cache).exist?(:cats)).to eq(false)
end
end
context 'with an existing repository' do
it 'caches the output' do
object = double
expect(object).to receive(:number).once.and_return(10)
2.times do
val = repository.cache_method_output(:cats) { object.number }
expect(val).to eq(10)
end
expect(repository.send(:cache).exist?(:cats)).to eq(true)
expect(repository.instance_variable_get(:@cats)).to eq(10)
end
end
end
describe '#refresh_method_caches' do describe '#refresh_method_caches' do
it 'refreshes the caches of the given types' do it 'refreshes the caches of the given types' do
expect(repository).to receive(:expire_method_caches) expect(repository).to receive(:expire_method_caches)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment