Commit 30954a07 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Improve HTTP caching for issue discussions

This skips rendering of notes to JSON and returns a 304 early when the
notes have not changed.
parent d9445328
......@@ -159,9 +159,9 @@ module IssuableActions
discussions = Discussion.build_collection(notes, issuable)
if issuable.is_a?(MergeRequest)
cache_context = [current_user&.cache_key, project.team.human_max_access(current_user&.id)].join(':')
render_cached(discussions, with: discussion_serializer, cache_context: -> (_) { cache_context }, context: self)
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)
render json: discussion_serializer.represent(discussions, context: self) if stale?(etag: [discussion_cache_context, discussions])
else
render json: discussion_serializer.represent(discussions, context: self)
end
......@@ -197,6 +197,10 @@ module IssuableActions
current_user&.user_preference&.previous_changes&.any?
end
def discussion_cache_context
[current_user&.cache_key, project.team.human_max_access(current_user&.id)].join(':')
end
def discussion_serializer
DiscussionSerializer.new(project: project, noteable: issuable, current_user: current_user, note_entity: ProjectNoteEntity)
end
......
---
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
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'issue discussions' do
describe 'GET /:namespace/:project/-/issues/:iid/discussions' do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:note_author) { create(:user) }
let_it_be(:notes) { create_list(:note, 5, project: project, noteable: issue, author: note_author) }
before_all do
project.add_maintainer(user)
end
def get_discussions
get discussions_namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: issue.iid), headers: {
'If-None-Match' => @etag
}
@etag = response.etag
end
before do
sign_in(user)
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)
get_discussions
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
it_behaves_like 'cache miss'
end
context 'when issue_discussions_http_cache is enabled' do
before do
stub_feature_flags(issue_discussions_http_cache: true)
end
it_behaves_like 'cache hit'
context 'when user role changes' do
before do
project.add_guest(user)
end
it_behaves_like 'cache miss'
end
context 'when emoji is awarded to a note' do
before do
travel_to(1.minute.from_now) { create(:award_emoji, awardable: notes.first) }
end
it_behaves_like 'cache miss'
end
context 'when note author name changes' do
before do
note_author.update!(name: 'New name')
end
it_behaves_like 'cache miss'
end
context 'when note author status changes' do
before do
Users::SetStatusService.new(note_author, message: "updated status").execute
end
it_behaves_like 'cache miss'
end
context 'when note author role changes' do
before do
project.add_developer(note_author)
end
it_behaves_like 'cache miss'
end
context 'when note is added' do
before do
create(:note, project: project, noteable: issue)
end
it_behaves_like 'cache miss'
end
context 'when note is modified' do
before do
notes.first.update!(note: 'edited text')
end
it_behaves_like 'cache miss'
end
context 'when note is deleted' do
before do
notes.first.destroy!
end
it_behaves_like 'cache miss'
end
end
end
end
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