Merge branch 'add-caching-to-archive-endpoint' into 'master'
Return an ETag header for the archive endpoint See merge request gitlab-org/gitlab-ce!30581
This commit is contained in:
commit
4fc9254f20
4 changed files with 108 additions and 7 deletions
|
@ -6,6 +6,7 @@ class Projects::RepositoriesController < Projects::ApplicationController
|
|||
# Authorize
|
||||
before_action :require_non_empty_project, except: :create
|
||||
before_action :assign_archive_vars, only: :archive
|
||||
before_action :assign_append_sha, only: :archive
|
||||
before_action :authorize_download_code!
|
||||
before_action :authorize_admin_project!, only: :create
|
||||
|
||||
|
@ -16,19 +17,64 @@ class Projects::RepositoriesController < Projects::ApplicationController
|
|||
end
|
||||
|
||||
def archive
|
||||
append_sha = params[:append_sha]
|
||||
set_cache_headers
|
||||
return if archive_not_modified?
|
||||
|
||||
if @ref
|
||||
shortname = "#{@project.path}-#{@ref.tr('/', '-')}"
|
||||
append_sha = false if @filename == shortname
|
||||
end
|
||||
|
||||
send_git_archive @repository, ref: @ref, path: params[:path], format: params[:format], append_sha: append_sha
|
||||
send_git_archive @repository, **repo_params
|
||||
rescue => ex
|
||||
logger.error("#{self.class.name}: #{ex}")
|
||||
git_not_found!
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def repo_params
|
||||
@repo_params ||= { ref: @ref, path: params[:path], format: params[:format], append_sha: @append_sha }
|
||||
end
|
||||
|
||||
def set_cache_headers
|
||||
expires_in cache_max_age(archive_metadata['CommitId']), public: project.public?
|
||||
fresh_when(etag: archive_metadata['ArchivePath'])
|
||||
end
|
||||
|
||||
def archive_not_modified?
|
||||
# Check response freshness (Last-Modified and ETag)
|
||||
# against request If-Modified-Since and If-None-Match conditions.
|
||||
request.fresh?(response)
|
||||
end
|
||||
|
||||
def archive_metadata
|
||||
@archive_metadata ||= @repository.archive_metadata(
|
||||
@ref,
|
||||
'', # Where archives are stored isn't really important for ETag purposes
|
||||
repo_params[:format],
|
||||
path: repo_params[:path],
|
||||
append_sha: @append_sha
|
||||
)
|
||||
end
|
||||
|
||||
def cache_max_age(commit_id)
|
||||
if @ref == commit_id
|
||||
# This is a link to an archive by a commit SHA. That means that the archive
|
||||
# is immutable. The only reason to invalidate the cache is if the commit
|
||||
# was deleted or if the user lost access to the repository.
|
||||
Repository::ARCHIVE_CACHE_TIME_IMMUTABLE
|
||||
else
|
||||
# A branch or tag points at this archive. That means that the expected archive
|
||||
# content may change over time.
|
||||
Repository::ARCHIVE_CACHE_TIME
|
||||
end
|
||||
end
|
||||
|
||||
def assign_append_sha
|
||||
@append_sha = params[:append_sha]
|
||||
|
||||
if @ref
|
||||
shortname = "#{@project.path}-#{@ref.tr('/', '-')}"
|
||||
@append_sha = false if @filename == shortname
|
||||
end
|
||||
end
|
||||
|
||||
def assign_archive_vars
|
||||
if params[:id]
|
||||
@ref, @filename = extract_ref(params[:id])
|
||||
|
|
|
@ -7,6 +7,9 @@ class Repository
|
|||
REF_KEEP_AROUND = 'keep-around'.freeze
|
||||
REF_ENVIRONMENTS = 'environments'.freeze
|
||||
|
||||
ARCHIVE_CACHE_TIME = 60 # Cache archives referred to by a (mutable) ref for 1 minute
|
||||
ARCHIVE_CACHE_TIME_IMMUTABLE = 3600 # Cache archives referred to by an immutable reference for 1 hour
|
||||
|
||||
RESERVED_REFS_NAMES = %W[
|
||||
heads
|
||||
tags
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Return an ETag header for the archive endpoint
|
||||
merge_request: 30581
|
||||
author:
|
||||
type: added
|
|
@ -77,6 +77,53 @@ describe Projects::RepositoriesController do
|
|||
expect(response).to have_gitlab_http_status(404)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'caching' do
|
||||
it 'sets appropriate caching headers' do
|
||||
get_archive
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(response.header['ETag']).to be_present
|
||||
expect(response.header['Cache-Control']).to include('max-age=60, private')
|
||||
end
|
||||
|
||||
context 'when project is public' do
|
||||
let(:project) { create(:project, :repository, :public) }
|
||||
|
||||
it 'sets appropriate caching headers' do
|
||||
get_archive
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(response.header['ETag']).to be_present
|
||||
expect(response.header['Cache-Control']).to include('max-age=60, public')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when ref is a commit SHA' do
|
||||
it 'max-age is set to 3600 in Cache-Control header' do
|
||||
get_archive('ddd0f15ae83993f5cb66a927a28673882e99100b')
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(response.header['Cache-Control']).to include('max-age=3600')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when If-None-Modified header is set' do
|
||||
it 'returns a 304 status' do
|
||||
# Get the archive cached first
|
||||
get_archive
|
||||
|
||||
request.headers['If-None-Match'] = response.headers['ETag']
|
||||
get_archive
|
||||
|
||||
expect(response).to have_gitlab_http_status(304)
|
||||
end
|
||||
end
|
||||
|
||||
def get_archive(id = 'feature')
|
||||
get :archive, params: { namespace_id: project.namespace, project_id: project, id: id }, format: 'zip'
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue