Commit 0ce91537 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'cache-raw-endpoints-for-logged-in-users' into 'master'

Enable caching of Git blobs even when signed in

See merge request gitlab-org/gitlab!47430
parents 192b7064 ed09090b
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
class Projects::AvatarsController < Projects::ApplicationController class Projects::AvatarsController < Projects::ApplicationController
include SendsBlob include SendsBlob
skip_before_action :default_cache_headers, only: :show
before_action :authorize_admin_project!, only: [:destroy] before_action :authorize_admin_project!, only: [:destroy]
feature_category :projects feature_category :projects
......
...@@ -6,6 +6,8 @@ class Projects::RawController < Projects::ApplicationController ...@@ -6,6 +6,8 @@ class Projects::RawController < Projects::ApplicationController
include SendsBlob include SendsBlob
include StaticObjectExternalStorage include StaticObjectExternalStorage
skip_before_action :default_cache_headers, only: :show
prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:blob) } prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:blob) }
before_action :require_non_empty_project before_action :require_non_empty_project
......
...@@ -8,6 +8,8 @@ class Projects::RepositoriesController < Projects::ApplicationController ...@@ -8,6 +8,8 @@ class Projects::RepositoriesController < Projects::ApplicationController
prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) } prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) }
skip_before_action :default_cache_headers, only: :archive
# Authorize # Authorize
before_action :require_non_empty_project, except: :create before_action :require_non_empty_project, except: :create
before_action :archive_rate_limit!, only: :archive before_action :archive_rate_limit!, only: :archive
......
---
title: Enable HTTP caching of repository raw, archive, and avatar endpoints
merge_request: 47430
author:
type: performance
...@@ -3,14 +3,14 @@ ...@@ -3,14 +3,14 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Projects::AvatarsController do RSpec.describe Projects::AvatarsController do
let_it_be(:project) { create(:project, :repository) } describe 'GET #show' do
let_it_be(:project) { create(:project, :public, :repository) }
before do before do
controller.instance_variable_set(:@project, project) controller.instance_variable_set(:@project, project)
end end
describe 'GET #show' do subject { get :show, params: { namespace_id: project.namespace, project_id: project.path } }
subject { get :show, params: { namespace_id: project.namespace, project_id: project } }
context 'when repository has no avatar' do context 'when repository has no avatar' do
it 'shows 404' do it 'shows 404' do
...@@ -37,6 +37,15 @@ RSpec.describe Projects::AvatarsController do ...@@ -37,6 +37,15 @@ RSpec.describe Projects::AvatarsController do
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq 'true' expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq 'true'
end end
it 'sets appropriate caching headers' do
sign_in(project.owner)
subject
expect(response.cache_control[:public]).to eq(true)
expect(response.cache_control[:max_age]).to eq(60)
expect(response.cache_control[:no_store]).to be_nil
end
it_behaves_like 'project cache control headers' it_behaves_like 'project cache control headers'
end end
...@@ -51,9 +60,16 @@ RSpec.describe Projects::AvatarsController do ...@@ -51,9 +60,16 @@ RSpec.describe Projects::AvatarsController do
end end
describe 'DELETE #destroy' do describe 'DELETE #destroy' do
let(:project) { create(:project, :repository, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) }
before do
sign_in(project.owner)
end
it 'removes avatar from DB by calling destroy' do it 'removes avatar from DB by calling destroy' do
delete :destroy, params: { namespace_id: project.namespace.id, project_id: project.id } delete :destroy, params: { namespace_id: project.namespace.path, project_id: project.path }
expect(response).to redirect_to(edit_project_path(project, anchor: 'js-general-project-settings'))
expect(project.avatar.present?).to be_falsey expect(project.avatar.present?).to be_falsey
expect(project).to be_valid expect(project).to be_valid
end end
......
...@@ -226,6 +226,15 @@ RSpec.describe Projects::RawController do ...@@ -226,6 +226,15 @@ RSpec.describe Projects::RawController do
get(:show, params: { namespace_id: project.namespace, project_id: project, id: 'master/README.md' }) get(:show, params: { namespace_id: project.namespace, project_id: project, id: 'master/README.md' })
end end
it 'sets appropriate caching headers' do
sign_in create(:user)
request_file
expect(response.cache_control[:public]).to eq(true)
expect(response.cache_control[:max_age]).to eq(60)
expect(response.cache_control[:no_store]).to be_nil
end
context 'when If-None-Match header is set' do context 'when If-None-Match header is set' do
it 'returns a 304 status' do it 'returns a 304 status' do
request_file request_file
......
...@@ -122,7 +122,9 @@ RSpec.describe Projects::RepositoriesController do ...@@ -122,7 +122,9 @@ RSpec.describe Projects::RepositoriesController do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response.header['ETag']).to be_present expect(response.header['ETag']).to be_present
expect(response.header['Cache-Control']).to include('max-age=60, private') expect(response.cache_control[:public]).to eq(false)
expect(response.cache_control[:max_age]).to eq(60)
expect(response.cache_control[:no_store]).to be_nil
end end
context 'when project is public' do context 'when project is public' 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