Commit 99cdf1d0 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'cached-branches-api' into 'master'

Add caching to branches API [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!61157
parents 9fc8ef00 febd38fc
---
name: api_caching_branches
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61157
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/330371
milestone: '13.12'
type: development
group: group::source code
default_enabled: false
......@@ -47,13 +47,25 @@ module API
merged_branch_names = repository.merged_branch_names(branches.map(&:name))
present(
branches,
with: Entities::Branch,
current_user: current_user,
project: user_project,
merged_branch_names: merged_branch_names
)
if Feature.enabled?(:api_caching_branches, user_project, type: :development, default_enabled: :yaml)
present_cached(
branches,
with: Entities::Branch,
current_user: current_user,
project: user_project,
merged_branch_names: merged_branch_names,
expires_in: 10.minutes,
cache_context: -> (branch) { [current_user&.cache_key, merged_branch_names.include?(branch.name)] }
)
else
present(
branches,
with: Entities::Branch,
current_user: current_user,
project: user_project,
merged_branch_names: merged_branch_names
)
end
end
resource ':id/repository/branches/:branch', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
......
......@@ -28,6 +28,10 @@ module Gitlab
def state
active? ? :active : :stale
end
def cache_key
"branch:" + Digest::SHA1.hexdigest([name, target, dereferenced_target&.sha].join(':'))
end
end
end
end
......@@ -44,6 +44,16 @@ RSpec.describe Gitlab::Git::Branch, :seed_helper do
end
end
describe "#cache_key" do
subject { repository.branches.first }
it "returns a cache key that changes based on changeable values" do
digest = Digest::SHA1.hexdigest([subject.name, subject.target, subject.dereferenced_target.sha].join(":"))
expect(subject.cache_key).to eq("branch:#{digest}")
end
end
describe '#size' do
subject { super().size }
......
......@@ -53,7 +53,7 @@ RSpec.describe API::Branches do
end
it 'determines only a limited number of merged branch names' do
expect(API::Entities::Branch).to receive(:represent).with(anything, has_up_to_merged_branch_names_count(2)).and_call_original
expect(API::Entities::Branch).to receive(:represent).with(anything, has_up_to_merged_branch_names_count(2)).at_least(:once).and_call_original
get api(route, current_user), params: base_params.merge(per_page: 2)
......@@ -111,7 +111,7 @@ RSpec.describe API::Branches do
end
it 'determines only a limited number of merged branch names' do
expect(API::Entities::Branch).to receive(:represent).with(anything, has_up_to_merged_branch_names_count(2)).and_call_original
expect(API::Entities::Branch).to receive(:represent).with(anything, has_up_to_merged_branch_names_count(2)).at_least(:once).and_call_original
get api(route, current_user), params: base_params.merge(per_page: 2)
......
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