Commit dcd69cbf authored by Shinya Maeda's avatar Shinya Maeda

Release API caching V2

This commit introduces a caching on the Release LIST v4 API
in order to avoid repeated computation on the payload rendering.
parent 694ce654
...@@ -5,7 +5,7 @@ module Releases ...@@ -5,7 +5,7 @@ module Releases
include ShaAttribute include ShaAttribute
include Presentable include Presentable
belongs_to :release, inverse_of: :evidences belongs_to :release, inverse_of: :evidences, touch: true
default_scope { order(created_at: :asc) } # rubocop:disable Cop/DefaultScope default_scope { order(created_at: :asc) } # rubocop:disable Cop/DefaultScope
......
...@@ -4,7 +4,7 @@ module Releases ...@@ -4,7 +4,7 @@ module Releases
class Link < ApplicationRecord class Link < ApplicationRecord
self.table_name = 'release_links' self.table_name = 'release_links'
belongs_to :release belongs_to :release, touch: true
# See https://gitlab.com/gitlab-org/gitlab/-/issues/218753 # See https://gitlab.com/gitlab-org/gitlab/-/issues/218753
# Regex modified to prevent catastrophic backtracking # Regex modified to prevent catastrophic backtracking
......
---
name: api_caching_releases
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60194
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329861
milestone: '13.12'
type: development
group: group::release
default_enabled: false
...@@ -119,8 +119,11 @@ module API ...@@ -119,8 +119,11 @@ module API
objs.flatten! objs.flatten!
map = multi_key_map(objs, context: context) map = multi_key_map(objs, context: context)
cache.fetch_multi(*map.keys, **kwargs) do |key| # TODO: `contextual_cache_key` should be constructed based on the guideline https://docs.gitlab.com/ee/development/redis.html#multi-key-commands.
yield map[key] Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
cache.fetch_multi(*map.keys, **kwargs) do |key|
yield map[key]
end
end end
end end
......
...@@ -33,7 +33,21 @@ module API ...@@ -33,7 +33,21 @@ module API
get ':id/releases' do get ':id/releases' do
releases = ::ReleasesFinder.new(user_project, current_user, declared_params.slice(:order_by, :sort)).execute releases = ::ReleasesFinder.new(user_project, current_user, declared_params.slice(:order_by, :sort)).execute
present paginate(releases), with: Entities::Release, current_user: current_user if Feature.enabled?(:api_caching_releases, user_project, default_enabled: :yaml)
# We cache the serialized payload per user in order to avoid repeated renderings.
# Since the cached result could contain sensitive information,
# it will expire in a short interval.
present_cached paginate(releases),
with: Entities::Release,
# `current_user` could be absent if the releases are publicly accesible.
# We should not use `cache_key` for the user because the version/updated_at
# context is unnecessary here.
cache_context: -> (_) { "user:{#{current_user&.id}}" },
expires_in: 5.minutes,
current_user: current_user
else
present paginate(releases), with: Entities::Release, current_user: current_user
end
end end
desc 'Get a single project release' do desc 'Get a single project release' do
......
...@@ -18,7 +18,7 @@ RSpec.describe API::Releases do ...@@ -18,7 +18,7 @@ RSpec.describe API::Releases do
project.add_developer(developer) project.add_developer(developer)
end end
describe 'GET /projects/:id/releases' do describe 'GET /projects/:id/releases', :use_clean_rails_redis_caching do
context 'when there are two releases' do context 'when there are two releases' do
let!(:release_1) do let!(:release_1) do
create(:release, create(:release,
...@@ -147,6 +147,60 @@ RSpec.describe API::Releases do ...@@ -147,6 +147,60 @@ RSpec.describe API::Releases do
end.not_to exceed_all_query_limit(control_count) end.not_to exceed_all_query_limit(control_count)
end end
it 'serializes releases for the first time and read cached data from the second time' do
create_list(:release, 2, project: project)
expect(API::Entities::Release)
.to receive(:represent).with(instance_of(Release), any_args)
.twice
5.times { get api("/projects/#{project.id}/releases", maintainer) }
end
it 'increments the cache key when link is updated' do
releases = create_list(:release, 2, project: project)
expect(API::Entities::Release)
.to receive(:represent).with(instance_of(Release), any_args)
.exactly(4).times
2.times { get api("/projects/#{project.id}/releases", maintainer) }
releases.each { |release| create(:release_link, release: release) }
3.times { get api("/projects/#{project.id}/releases", maintainer) }
end
it 'increments the cache key when evidence is updated' do
releases = create_list(:release, 2, project: project)
expect(API::Entities::Release)
.to receive(:represent).with(instance_of(Release), any_args)
.exactly(4).times
2.times { get api("/projects/#{project.id}/releases", maintainer) }
releases.each { |release| create(:evidence, release: release) }
3.times { get api("/projects/#{project.id}/releases", maintainer) }
end
context 'when api_caching_releases feature flag is disabled' do
before do
stub_feature_flags(api_caching_releases: false)
end
it 'serializes releases everytime' do
create_list(:release, 2, project: project)
expect(API::Entities::Release)
.to receive(:represent).with(kind_of(ActiveRecord::Relation), any_args)
.exactly(5).times
5.times { get api("/projects/#{project.id}/releases", maintainer) }
end
end
context 'when tag does not exist in git repository' do context 'when tag does not exist in git repository' do
let!(:release) { create(:release, project: project, tag: 'v1.1.5') } let!(:release) { create(:release, project: project, tag: 'v1.1.5') }
...@@ -230,6 +284,20 @@ RSpec.describe API::Releases do ...@@ -230,6 +284,20 @@ RSpec.describe API::Releases do
end end
end end
end end
context 'when releases are public and request user is absent' do
let(:project) { create(:project, :repository, :public) }
it 'returns the releases' do
create(:release, project: project, tag: 'v0.1')
get api("/projects/#{project.id}/releases")
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.count).to eq(1)
expect(json_response.first['tag_name']).to eq('v0.1')
end
end
end end
describe 'GET /projects/:id/releases/:tag_name' do describe 'GET /projects/:id/releases/: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