Commit 4c550234 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Improve ETag caching for issue discussions

This removes the issue_discussions_http_cache feature flag. This allows
us to avoid JSON serialization when the ETag matches.

This is working as expected and has been tested on GitLab.com for 3
weeks already.

Changelog: performance
parent 39dcf147
......@@ -160,7 +160,7 @@ module IssuableActions
if issuable.is_a?(MergeRequest)
render_cached(discussions, with: discussion_serializer, cache_context: -> (_) { discussion_cache_context }, context: self)
elsif issuable.is_a?(Issue) && Feature.enabled?(:issue_discussions_http_cache, default_enabled: :yaml)
elsif issuable.is_a?(Issue)
render json: discussion_serializer.represent(discussions, context: self) if stale?(etag: [discussion_cache_context, discussions])
else
render json: discussion_serializer.represent(discussions, context: self)
......
---
name: issue_discussions_http_cache
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72589
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343309
milestone: '14.5'
type: development
group: group::project management
default_enabled: false
......@@ -14,6 +14,7 @@ RSpec.describe 'issue discussions' do
project.add_maintainer(user)
end
context 'HTTP caching' do
def get_discussions
get discussions_namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: issue.iid), headers: {
'If-None-Match' => @etag
......@@ -28,17 +29,6 @@ RSpec.describe 'issue discussions' do
get_discussions
end
shared_examples 'cache miss' do
it 'returns 200 and serializes JSON' do
expect(DiscussionSerializer).to receive(:new).and_call_original
get_discussions
expect(response).to have_gitlab_http_status(:ok)
end
end
shared_examples 'cache hit' do
it 'returns 304 without serializing JSON' do
expect(DiscussionSerializer).not_to receive(:new)
......@@ -46,22 +36,16 @@ RSpec.describe 'issue discussions' do
expect(response).to have_gitlab_http_status(:not_modified)
end
end
context 'when issue_discussions_http_cache is disabled' do
before do
stub_feature_flags(issue_discussions_http_cache: false)
end
shared_examples 'cache miss' do
it 'returns 200 and serializes JSON' do
expect(DiscussionSerializer).to receive(:new).and_call_original
it_behaves_like 'cache miss'
end
get_discussions
context 'when issue_discussions_http_cache is enabled' do
before do
stub_feature_flags(issue_discussions_http_cache: true)
expect(response).to have_gitlab_http_status(:ok)
end
end
it_behaves_like 'cache hit'
context 'when user role changes' do
before 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