Commit 59ea5786 authored by Robert May's avatar Robert May

API "view" caching for tags

Caches generated JSON for objects, in this instance tags,
and bypasses the Grape output to return it directly.
parent 417f8d17
...@@ -4,7 +4,7 @@ module ProtectedRef ...@@ -4,7 +4,7 @@ module ProtectedRef
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
belongs_to :project belongs_to :project, touch: true
validates :name, presence: true validates :name, presence: true
validates :project, presence: true validates :project, presence: true
......
...@@ -8,7 +8,7 @@ class Release < ApplicationRecord ...@@ -8,7 +8,7 @@ class Release < ApplicationRecord
cache_markdown_field :description cache_markdown_field :description
belongs_to :project belongs_to :project, touch: true
# releases prior to 11.7 have no author # releases prior to 11.7 have no author
belongs_to :author, class_name: 'User' belongs_to :author, class_name: 'User'
......
---
title: API JSON caching for tags endpoint
merge_request: 54975
author:
type: performance
---
name: api_caching_tags
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54975
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/324391
milestone: '13.10'
type: development
group: group::source code
default_enabled: false
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module API module API
module Helpers module Helpers
include Gitlab::Utils include Gitlab::Utils
include Helpers::Caching
include Helpers::Pagination include Helpers::Pagination
include Helpers::PaginationStrategies include Helpers::PaginationStrategies
......
# frozen_string_literal: true
# Grape helpers for caching.
#
# This module helps introduce standardised caching into the Grape API
# in a similar manner to the standard Grape DSL.
module API
module Helpers
module Caching
# @return [ActiveSupport::Duration]
DEFAULT_EXPIRY = 1.day
# @return [ActiveSupport::Cache::Store]
def cache
Rails.cache
end
# This is functionally equivalent to the standard `#present` used in
# Grape endpoints, but the JSON for the object, or for each object of
# a collection, will be cached.
#
# With a collection all the keys will be fetched in a single call and the
# Entity rendered for those missing from the cache, which are then written
# back into it.
#
# Both the single object, and all objects inside a collection, must respond
# to `#cache_key`.
#
# To override the Grape formatter we return a custom wrapper in
# `Gitlab::Json::PrecompiledJson` which tells the `Gitlab::Json::GrapeFormatter`
# to export the string without conversion.
#
# A cache context can be supplied to add more context to the cache key. This
# defaults to including the `current_user` in every key for safety, unless overridden.
#
# @param obj_or_collection [Object, Enumerable<Object>] the object or objects to render
# @param with [Grape::Entity] the entity to use for rendering
# @param cache_context [Proc] a proc to call for each object to provide more context to the cache key
# @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
# @param presenter_args [Hash] keyword arguments to be passed to the entity
# @return [Gitlab::Json::PrecompiledJson]
def present_cached(obj_or_collection, with:, cache_context: -> (_) { current_user.cache_key }, expires_in: DEFAULT_EXPIRY, **presenter_args)
json =
if obj_or_collection.is_a?(Enumerable)
cached_collection(
obj_or_collection,
presenter: with,
presenter_args: presenter_args,
context: cache_context,
expires_in: expires_in
)
else
cached_object(
obj_or_collection,
presenter: with,
presenter_args: presenter_args,
context: cache_context,
expires_in: expires_in
)
end
body Gitlab::Json::PrecompiledJson.new(json)
end
private
# Optionally uses a `Proc` to add context to a cache key
#
# @param object [Object] must respond to #cache_key
# @param context [Proc] a proc that will be called with the object as an argument, and which should return a
# string or array of strings to be combined into the cache key
# @return [String]
def contextual_cache_key(object, context)
return object.cache_key if context.nil?
[object.cache_key, context.call(object)].flatten.join(":")
end
# Used for fetching or rendering a single object
#
# @param object [Object] the object to render
# @param presenter [Grape::Entity]
# @param presenter_args [Hash] keyword arguments to be passed to the entity
# @param context [Proc]
# @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
# @return [String]
def cached_object(object, presenter:, presenter_args:, context:, expires_in:)
cache.fetch(contextual_cache_key(object, context), expires_in: expires_in) do
Gitlab::Json.dump(presenter.represent(object, **presenter_args).as_json)
end
end
# Used for fetching or rendering multiple objects
#
# @param objects [Enumerable<Object>] the objects to render
# @param presenter [Grape::Entity]
# @param presenter_args [Hash] keyword arguments to be passed to the entity
# @param context [Proc]
# @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
# @return [Array<String>]
def cached_collection(collection, presenter:, presenter_args:, context:, expires_in:)
json = fetch_multi(collection, context: context, expires_in: expires_in) do |obj|
Gitlab::Json.dump(presenter.represent(obj, **presenter_args).as_json)
end
json.values
end
# An adapted version of ActiveSupport::Cache::Store#fetch_multi.
#
# The original method only provides the missing key to the block,
# not the missing object, so we have to create a map of cache keys
# to the objects to allow us to pass the object to the missing value
# block.
#
# The result is that this is functionally identical to `#fetch`.
def fetch_multi(*objs, context:, **kwargs)
objs.flatten!
map = multi_key_map(objs, context: context)
cache.fetch_multi(*map.keys, **kwargs) do |key|
yield map[key]
end
end
# @param objects [Enumerable<Object>] objects which _must_ respond to `#cache_key`
# @param context [Proc] a proc that can be called to help generate each cache key
# @return [Hash]
def multi_key_map(objects, context:)
objects.index_by do |object|
contextual_cache_key(object, context)
end
end
end
end
end
...@@ -28,7 +28,13 @@ module API ...@@ -28,7 +28,13 @@ module API
sort: "#{params[:order_by]}_#{params[:sort]}", sort: "#{params[:order_by]}_#{params[:sort]}",
search: params[:search]).execute search: params[:search]).execute
present paginate(::Kaminari.paginate_array(tags)), with: Entities::Tag, project: user_project paginated_tags = paginate(::Kaminari.paginate_array(tags))
if Feature.enabled?(:api_caching_tags, user_project, type: :development)
present_cached paginated_tags, with: Entities::Tag, project: user_project, cache_context: -> (_tag) { user_project.cache_key }
else
present paginated_tags, with: Entities::Tag, project: user_project
end
end end
desc 'Get a single repository tag' do desc 'Get a single repository tag' do
......
...@@ -87,6 +87,10 @@ module Gitlab ...@@ -87,6 +87,10 @@ module Gitlab
end end
end end
def cache_key
"tag:" + Digest::SHA1.hexdigest([name, message, target, target_commit&.sha].join)
end
private private
def message_from_gitaly_tag def message_from_gitaly_tag
......
...@@ -186,9 +186,14 @@ module Gitlab ...@@ -186,9 +186,14 @@ module Gitlab
# The `env` param is ignored because it's not needed in either our formatter or Grape's, # The `env` param is ignored because it's not needed in either our formatter or Grape's,
# but it is passed through for consistency. # but it is passed through for consistency.
# #
# If explicitly supplied with a `PrecompiledJson` instance it will skip conversion
# and return it directly. This is mostly used in caching.
#
# @param object [Object] # @param object [Object]
# @return [String] # @return [String]
def self.call(object, env = nil) def self.call(object, env = nil)
return object.to_s if object.is_a?(PrecompiledJson)
if Feature.enabled?(:grape_gitlab_json, default_enabled: true) if Feature.enabled?(:grape_gitlab_json, default_enabled: true)
Gitlab::Json.dump(object) Gitlab::Json.dump(object)
else else
...@@ -197,6 +202,34 @@ module Gitlab ...@@ -197,6 +202,34 @@ module Gitlab
end end
end end
# Wrapper class used to skip JSON dumping on Grape endpoints.
class PrecompiledJson
UnsupportedFormatError = Class.new(StandardError)
# @overload PrecompiledJson.new("foo")
# @param value [String]
#
# @overload PrecompiledJson.new(["foo", "bar"])
# @param value [Array<String>]
def initialize(value)
@value = value
end
# Convert the value to a String. This will invoke
# `#to_s` on the members of the value if it's an array.
#
# @return [String]
# @raise [NoMethodError] if the objects in an array doesn't support to_s
# @raise [PrecompiledJson::UnsupportedFormatError] if the value is neither a String or Array
def to_s
return @value if @value.is_a?(String)
return "[#{@value.join(',')}]" if @value.is_a?(Array)
raise UnsupportedFormatError
end
end
class LimitedEncoder class LimitedEncoder
LimitExceeded = Class.new(StandardError) LimitExceeded = Class.new(StandardError)
......
...@@ -65,14 +65,23 @@ RSpec.describe 'factories' do ...@@ -65,14 +65,23 @@ RSpec.describe 'factories' do
# associations must be unique and cannot be reused, or the factory default # associations must be unique and cannot be reused, or the factory default
# is being mutated. # is being mutated.
skip_factory_defaults = %i[ skip_factory_defaults = %i[
evidence
exported_protected_branch
fork_network_member fork_network_member
group_member group_member
import_state import_state
milestone_release
namespace namespace
project_broken_repo project_broken_repo
prometheus_alert prometheus_alert
prometheus_alert_event prometheus_alert_event
prometheus_metric prometheus_metric
protected_branch
protected_branch_merge_access_level
protected_branch_push_access_level
protected_tag
release
release_link
self_managed_prometheus_alert_event self_managed_prometheus_alert_event
users_star_project users_star_project
wiki_page wiki_page
......
# frozen_string_literal: true
require "spec_helper"
RSpec.describe API::Helpers::Caching do
subject(:instance) { Class.new.include(described_class).new }
describe "#present_cached" do
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) }
let(:presenter) { API::Entities::Todo }
let(:kwargs) do
{
with: presenter,
project: project
}
end
subject do
instance.present_cached(presentable, **kwargs)
end
before do
# We have to stub #body as it's a Grape method
# unavailable in the module by itself
expect(instance).to receive(:body) do |data|
data
end
allow(instance).to receive(:current_user) { user }
end
context "single object" do
let_it_be(:presentable) { create(:todo, project: project) }
it { is_expected.to be_a(Gitlab::Json::PrecompiledJson) }
it "uses the presenter" do
expect(presenter).to receive(:represent).with(presentable, project: project)
subject
end
it "is valid JSON" do
parsed = Gitlab::Json.parse(subject.to_s)
expect(parsed).to be_a(Hash)
expect(parsed["id"]).to eq(presentable.id)
end
it "fetches from the cache" do
expect(instance.cache).to receive(:fetch).with("#{presentable.cache_key}:#{user.cache_key}", expires_in: described_class::DEFAULT_EXPIRY).once
subject
end
context "when a cache context is supplied" do
before do
kwargs[:cache_context] = -> (todo) { todo.project.cache_key }
end
it "uses the context to augment the cache key" do
expect(instance.cache).to receive(:fetch).with("#{presentable.cache_key}:#{project.cache_key}", expires_in: described_class::DEFAULT_EXPIRY).once
subject
end
end
context "when expires_in is supplied" do
it "sets the expiry when accessing the cache" do
kwargs[:expires_in] = 7.days
expect(instance.cache).to receive(:fetch).with("#{presentable.cache_key}:#{user.cache_key}", expires_in: 7.days).once
subject
end
end
end
context "for a collection of objects" do
let_it_be(:presentable) { Array.new(5).map { create(:todo, project: project) } }
it { is_expected.to be_an(Gitlab::Json::PrecompiledJson) }
it "uses the presenter" do
presentable.each do |todo|
expect(presenter).to receive(:represent).with(todo, project: project)
end
subject
end
it "is valid JSON" do
parsed = Gitlab::Json.parse(subject.to_s)
expect(parsed).to be_an(Array)
presentable.each_with_index do |todo, i|
expect(parsed[i]["id"]).to eq(todo.id)
end
end
it "fetches from the cache" do
keys = presentable.map { |todo| "#{todo.cache_key}:#{user.cache_key}" }
expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: described_class::DEFAULT_EXPIRY).once.and_call_original
subject
end
context "when a cache context is supplied" do
before do
kwargs[:cache_context] = -> (todo) { todo.project.cache_key }
end
it "uses the context to augment the cache key" do
keys = presentable.map { |todo| "#{todo.cache_key}:#{project.cache_key}" }
expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: described_class::DEFAULT_EXPIRY).once.and_call_original
subject
end
end
context "expires_in is supplied" do
it "sets the expiry when accessing the cache" do
keys = presentable.map { |todo| "#{todo.cache_key}:#{user.cache_key}" }
kwargs[:expires_in] = 7.days
expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: 7.days).once.and_call_original
subject
end
end
end
end
end
...@@ -101,4 +101,17 @@ RSpec.describe Gitlab::Git::Tag, :seed_helper do ...@@ -101,4 +101,17 @@ RSpec.describe Gitlab::Git::Tag, :seed_helper do
end end
end end
end end
describe "#cache_key" do
subject { repository.tags.first }
it "returns a cache key that changes based on changeable values" do
expect(subject).to receive(:name).and_return("v1.0.0")
expect(subject).to receive(:message).and_return("Initial release")
digest = Digest::SHA1.hexdigest(["v1.0.0", "Initial release", subject.target, subject.target_commit.sha].join)
expect(subject.cache_key).to eq("tag:#{digest}")
end
end
end end
...@@ -348,6 +348,66 @@ RSpec.describe Gitlab::Json do ...@@ -348,6 +348,66 @@ RSpec.describe Gitlab::Json do
subject subject
end end
end end
context "precompiled JSON" do
let(:obj) { Gitlab::Json::PrecompiledJson.new(result) }
it "renders the string directly" do
expect(subject).to eq(result)
end
it "calls #to_s on the object" do
expect(obj).to receive(:to_s).once
subject
end
it "doesn't run the JSON formatter" do
expect(Gitlab::Json).not_to receive(:dump)
subject
end
end
end
describe Gitlab::Json::PrecompiledJson do
subject(:precompiled) { described_class.new(obj) }
describe "#to_s" do
subject { precompiled.to_s }
context "obj is a string" do
let(:obj) { "{}" }
it "returns a string" do
expect(subject).to eq("{}")
end
end
context "obj is an array" do
let(:obj) { ["{\"foo\": \"bar\"}", "{}"] }
it "returns a string" do
expect(subject).to eq("[{\"foo\": \"bar\"},{}]")
end
end
context "obj is an array of un-stringables" do
let(:obj) { [BasicObject.new] }
it "raises an error" do
expect { subject }.to raise_error(NoMethodError)
end
end
context "obj is something else" do
let(:obj) { {} }
it "raises an error" do
expect { subject }.to raise_error(described_class::UnsupportedFormatError)
end
end
end
end end
describe Gitlab::Json::LimitedEncoder do describe Gitlab::Json::LimitedEncoder do
......
...@@ -4,7 +4,7 @@ require 'spec_helper' ...@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe ProtectedTag do RSpec.describe ProtectedTag do
describe 'Associations' do describe 'Associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project).touch(true) }
end end
describe 'Validation' do describe 'Validation' do
......
...@@ -10,7 +10,7 @@ RSpec.describe Release do ...@@ -10,7 +10,7 @@ RSpec.describe Release do
it { expect(release).to be_valid } it { expect(release).to be_valid }
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project).touch(true) }
it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to belong_to(:author).class_name('User') }
it { is_expected.to have_many(:links).class_name('Releases::Link') } it { is_expected.to have_many(:links).class_name('Releases::Link') }
it { is_expected.to have_many(:milestones) } it { is_expected.to have_many(:milestones) }
......
...@@ -17,131 +17,197 @@ RSpec.describe API::Tags do ...@@ -17,131 +17,197 @@ RSpec.describe API::Tags do
end end
describe 'GET /projects/:id/repository/tags' do describe 'GET /projects/:id/repository/tags' do
let(:route) { "/projects/#{project_id}/repository/tags" } shared_examples "get repository tags" do
let(:route) { "/projects/#{project_id}/repository/tags" }
context 'sorting' do context 'sorting' do
let(:current_user) { user } let(:current_user) { user }
it 'sorts by descending order by default' do it 'sorts by descending order by default' do
get api(route, current_user) get api(route, current_user)
desc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date } desc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date }
desc_order_tags.reverse!.map! { |tag| tag.dereferenced_target.id } desc_order_tags.reverse!.map! { |tag| tag.dereferenced_target.id }
expect(json_response.map { |tag| tag['commit']['id'] }).to eq(desc_order_tags) expect(json_response.map { |tag| tag['commit']['id'] }).to eq(desc_order_tags)
end end
it 'sorts by ascending order if specified' do it 'sorts by ascending order if specified' do
get api("#{route}?sort=asc", current_user) get api("#{route}?sort=asc", current_user)
asc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date } asc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date }
asc_order_tags.map! { |tag| tag.dereferenced_target.id } asc_order_tags.map! { |tag| tag.dereferenced_target.id }
expect(json_response.map { |tag| tag['commit']['id'] }).to eq(asc_order_tags) expect(json_response.map { |tag| tag['commit']['id'] }).to eq(asc_order_tags)
end end
it 'sorts by name in descending order when requested' do it 'sorts by name in descending order when requested' do
get api("#{route}?order_by=name", current_user) get api("#{route}?order_by=name", current_user)
ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort.reverse ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort.reverse
expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name) expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name)
end end
it 'sorts by name in ascending order when requested' do it 'sorts by name in ascending order when requested' do
get api("#{route}?order_by=name&sort=asc", current_user) get api("#{route}?order_by=name&sort=asc", current_user)
ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort
expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name) expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name)
end
end end
end
context 'searching' do context 'searching' do
it 'only returns searched tags' do it 'only returns searched tags' do
get api("#{route}", user), params: { search: 'v1.1.0' } get api("#{route}", user), params: { search: 'v1.1.0' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers expect(response).to include_pagination_headers
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response.size).to eq(1) expect(json_response.size).to eq(1)
expect(json_response[0]['name']).to eq('v1.1.0') expect(json_response[0]['name']).to eq('v1.1.0')
end
end end
end
shared_examples_for 'repository tags' do shared_examples_for 'repository tags' do
it 'returns the repository tags' do it 'returns the repository tags' do
get api(route, current_user) get api(route, current_user)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/tags') expect(response).to match_response_schema('public_api/v4/tags')
expect(response).to include_pagination_headers expect(response).to include_pagination_headers
expect(json_response.map { |r| r['name'] }).to include(tag_name) expect(json_response.map { |r| r['name'] }).to include(tag_name)
end
context 'when repository is disabled' do
include_context 'disabled repository'
it_behaves_like '403 response' do
let(:request) { get api(route, current_user) }
end
end
end end
context 'when repository is disabled' do context 'when unauthenticated', 'and project is public' do
include_context 'disabled repository' let(:project) { create(:project, :public, :repository) }
it_behaves_like '403 response' do it_behaves_like 'repository tags'
let(:request) { get api(route, current_user) } end
context 'when unauthenticated', 'and project is private' do
it_behaves_like '404 response' do
let(:request) { get api(route) }
let(:message) { '404 Project Not Found' }
end end
end end
end
context 'when unauthenticated', 'and project is public' do context 'when authenticated', 'as a maintainer' do
let(:project) { create(:project, :public, :repository) } let(:current_user) { user }
it_behaves_like 'repository tags' it_behaves_like 'repository tags'
end
context 'when unauthenticated', 'and project is private' do context 'requesting with the escaped project full path' do
it_behaves_like '404 response' do let(:project_id) { CGI.escape(project.full_path) }
let(:request) { get api(route) }
let(:message) { '404 Project Not Found' } it_behaves_like 'repository tags'
end
end end
end
context 'when authenticated', 'as a maintainer' do context 'when authenticated', 'as a guest' do
let(:current_user) { user } it_behaves_like '403 response' do
let(:request) { get api(route, guest) }
end
end
it_behaves_like 'repository tags' context 'with releases' do
let(:description) { 'Awesome release!' }
context 'requesting with the escaped project full path' do let!(:release) do
let(:project_id) { CGI.escape(project.full_path) } create(:release,
:legacy,
project: project,
tag: tag_name,
description: description)
end
it_behaves_like 'repository tags' it 'returns an array of project tags with release info' do
get api(route, user)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/tags')
expect(response).to include_pagination_headers
expected_tag = json_response.find { |r| r['name'] == tag_name }
expect(expected_tag['message']).to eq(tag_message)
expect(expected_tag['release']['description']).to eq(description)
end
end end
end end
context 'when authenticated', 'as a guest' do context ":api_caching_tags flag enabled", :use_clean_rails_memory_store_caching do
it_behaves_like '403 response' do before do
let(:request) { get api(route, guest) } stub_feature_flags(api_caching_tags: true)
end end
end
context 'with releases' do it_behaves_like "get repository tags"
let(:description) { 'Awesome release!' }
let!(:release) do describe "cache expiry" do
create(:release, let(:route) { "/projects/#{project_id}/repository/tags" }
:legacy, let(:current_user) { user }
project: project,
tag: tag_name,
description: description)
end
it 'returns an array of project tags with release info' do before do
get api(route, user) # Set the cache
get api(route, current_user)
end
expect(response).to have_gitlab_http_status(:ok) it "is cached" do
expect(response).to match_response_schema('public_api/v4/tags') expect(API::Entities::Tag).not_to receive(:represent)
expect(response).to include_pagination_headers
get api(route, current_user)
end
shared_examples "cache expired" do
it "isn't cached" do
expect(API::Entities::Tag).to receive(:represent).exactly(3).times
get api(route, current_user)
end
end
context "when protected tag is changed" do
before do
create(:protected_tag, name: tag_name, project: project)
end
it_behaves_like "cache expired"
end
expected_tag = json_response.find { |r| r['name'] == tag_name } context "when release is changed" do
expect(expected_tag['message']).to eq(tag_message) before do
expect(expected_tag['release']['description']).to eq(description) create(:release, :legacy, project: project, tag: tag_name)
end
it_behaves_like "cache expired"
end
context "when project is changed" do
before do
project.touch
end
it_behaves_like "cache expired"
end
end end
end end
context ":api_caching_tags flag disabled" do
before do
stub_feature_flags(api_caching_tags: false)
end
it_behaves_like "get repository tags"
end
end end
describe 'GET /projects/:id/repository/tags/:tag_name' do describe 'GET /projects/:id/repository/tags/:tag_name' do
......
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