Commit 07d52415 authored by David Kim's avatar David Kim

Fix cache_context to include more variables

parent 076b96b6
...@@ -27,10 +27,10 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic ...@@ -27,10 +27,10 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
diff_options_hash[:paths] = params[:paths] if params[:paths] diff_options_hash[:paths] = params[:paths] if params[:paths]
diffs = @compare.diffs_in_batch(params[:page], params[:per_page], diff_options: diff_options_hash) diffs = @compare.diffs_in_batch(params[:page], params[:per_page], diff_options: diff_options_hash)
positions = @merge_request.note_positions_for_paths(diffs.diff_file_paths, current_user) unfoldable_positions = @merge_request.note_positions_for_paths(diffs.diff_file_paths, current_user).unfoldable
environment = @merge_request.environments_for(current_user, latest: true).last environment = @merge_request.environments_for(current_user, latest: true).last
diffs.unfold_diff_files(positions.unfoldable) diffs.unfold_diff_files(unfoldable_positions)
diffs.write_cache diffs.write_cache
options = { options = {
...@@ -43,10 +43,24 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic ...@@ -43,10 +43,24 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
} }
if diff_options_hash[:paths].blank? && Feature.enabled?(:diffs_batch_render_cached, project, default_enabled: :yaml) if diff_options_hash[:paths].blank? && Feature.enabled?(:diffs_batch_render_cached, project, default_enabled: :yaml)
# NOTE: Any variables that would affect the resulting json needs to be added to the cache_context to avoid stale cache issues.
cache_context = [
current_user&.cache_key,
environment&.cache_key,
unfoldable_positions.map(&:to_h),
diff_view,
params[:w],
params[:expanded],
params[:page],
params[:per_page],
options[:merge_ref_head_diff],
options[:allow_tree_conflicts]
]
render_cached( render_cached(
diffs, diffs,
with: PaginatedDiffSerializer.new(current_user: current_user), with: PaginatedDiffSerializer.new(current_user: current_user),
cache_context: -> (_) { [diff_view, params[:w], params[:expanded], params[:per_page], params[:page]] }, cache_context: -> (_) { [Digest::SHA256.hexdigest(cache_context.to_s)] },
**options **options
) )
else else
......
...@@ -76,6 +76,78 @@ RSpec.describe 'Merge Requests Diffs' do ...@@ -76,6 +76,78 @@ RSpec.describe 'Merge Requests Diffs' do
subject subject
end end
context 'with the different user' do
let(:another_user) { create(:user) }
before do
project.add_maintainer(another_user)
sign_in(another_user)
end
it_behaves_like 'serializes diffs with expected arguments' do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
let(:expected_options) { collection_arguments(total_pages: 20) }
end
end
context 'with a new unfoldable diff position' do
let(:unfoldable_position) do
create(:diff_position)
end
before do
expect_next_instance_of(Gitlab::Diff::PositionCollection) do |instance|
expect(instance)
.to receive(:unfoldable)
.and_return([unfoldable_position])
end
end
it_behaves_like 'serializes diffs with expected arguments' do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
let(:expected_options) { collection_arguments(total_pages: 20) }
end
end
context 'with a new environment' do
let(:environment) do
create(:environment, :available, project: project)
end
let!(:deployment) do
create(:deployment, :success, environment: environment, ref: merge_request.source_branch)
end
it_behaves_like 'serializes diffs with expected arguments' do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
let(:expected_options) { collection_arguments(total_pages: 20).merge(environment: environment) }
end
end
context 'with disabled display_merge_conflicts_in_diff feature' do
before do
stub_feature_flags(display_merge_conflicts_in_diff: false)
end
it_behaves_like 'serializes diffs with expected arguments' do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
let(:expected_options) { collection_arguments(total_pages: 20).merge(allow_tree_conflicts: false) }
end
end
context 'with diff_head option' do
subject { go(page: 0, per_page: 5, diff_head: true) }
before do
merge_request.create_merge_head_diff!
end
it_behaves_like 'serializes diffs with expected arguments' do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_ref_head_diff: true) }
end
end
context 'with the different pagination option' do context 'with the different pagination option' do
subject { go(page: 5, per_page: 5) } subject { go(page: 5, per_page: 5) }
......
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