From 679fc2fcb973ea2a7008a4bad9147dd3b8a7672c Mon Sep 17 00:00:00 2001 From: Jacob Cody Wimer Date: Mon, 2 Mar 2026 18:34:12 -0500 Subject: [PATCH] Ensuring good caching for the most popular pages. Added tests. --- .../schools/_wrestler_row_cells.html.erb | 13 ++ app/views/schools/show.html.erb | 56 ++---- app/views/tournaments/_bracket_final.html.erb | 24 +-- app/views/tournaments/_bracket_round.html.erb | 26 +-- .../tournaments/_team_score_row.html.erb | 6 + .../tournaments/_up_matches_board.html.erb | 9 +- .../tournaments/_up_matches_mat_row.html.erb | 3 +- .../_up_matches_unassigned_row.html.erb | 8 + app/views/tournaments/bracket.html.erb | 4 +- app/views/tournaments/team_scores.html.erb | 14 +- .../weights/_readonly_wrestler_row.html.erb | 10 ++ app/views/weights/show.html.erb | 59 +++---- test/controllers/school_show_cache_test.rb | 143 +++++++++++++++ .../tournament_pages_cache_test.rb | 164 ++++++++++++++++++ test/controllers/up_matches_cache_test.rb | 77 +++++++- test/controllers/weight_show_cache_test.rb | 107 ++++++++++++ 16 files changed, 609 insertions(+), 114 deletions(-) create mode 100644 app/views/schools/_wrestler_row_cells.html.erb create mode 100644 app/views/tournaments/_team_score_row.html.erb create mode 100644 app/views/tournaments/_up_matches_unassigned_row.html.erb create mode 100644 app/views/weights/_readonly_wrestler_row.html.erb create mode 100644 test/controllers/school_show_cache_test.rb create mode 100644 test/controllers/tournament_pages_cache_test.rb create mode 100644 test/controllers/weight_show_cache_test.rb diff --git a/app/views/schools/_wrestler_row_cells.html.erb b/app/views/schools/_wrestler_row_cells.html.erb new file mode 100644 index 0000000..30d13c0 --- /dev/null +++ b/app/views/schools/_wrestler_row_cells.html.erb @@ -0,0 +1,13 @@ +<% if local_assigns[:school_permission_key].present? %> + <% wrestler_path_with_key = wrestler_path(wrestler) %> + <% wrestler_path_with_key += "?school_permission_key=#{school_permission_key}" %> + <%= link_to wrestler.name, wrestler_path_with_key %> +<% else %> + <%= link_to wrestler.name, wrestler_path(wrestler) %> +<% end %> +<%= wrestler.weight.max %> +<%= wrestler.season_win %>-<%= wrestler.season_loss %> <%= wrestler.criteria %> +<%= wrestler.original_seed %> +<%= wrestler.total_team_points - wrestler.total_points_deducted %> +<%= "Yes" if wrestler.extra? %> +<%= wrestler.next_match_bout_number %> <%= wrestler.next_match_mat_name %> diff --git a/app/views/schools/show.html.erb b/app/views/schools/show.html.erb index 6bde5c4..58e67ff 100644 --- a/app/views/schools/show.html.erb +++ b/app/views/schools/show.html.erb @@ -54,19 +54,8 @@ <% @wrestlers.sort_by { |w| w.weight.max }.each do |wrestler| %> <% if params[:school_permission_key].present? %> - - - <% wrestler_path_with_key = wrestler_path(wrestler) %> - <% wrestler_path_with_key += "?school_permission_key=#{params[:school_permission_key]}" if params[:school_permission_key].present? %> - <%= link_to wrestler.name, wrestler_path_with_key %> - - <%= wrestler.weight.max %> - <%= wrestler.season_win %>-<%= wrestler.season_loss %> <%= wrestler.criteria %> - <%= wrestler.original_seed %> - <%= wrestler.total_team_points - wrestler.total_points_deducted %> - <%= "Yes" if wrestler.extra? %> - <%= wrestler.next_match_bout_number %> <%= wrestler.next_match_mat_name %> + <%= render "schools/wrestler_row_cells", wrestler: wrestler, school_permission_key: params[:school_permission_key] %> <% if can? :manage, wrestler.school %> @@ -86,34 +75,21 @@ <% end %> <% else %> - - <% cache ["#{wrestler.id}_school_show", @school] do %> - - <%= link_to wrestler.name, wrestler_path(wrestler) %> - <%= wrestler.weight.max %> - <%= wrestler.season_win %>-<%= wrestler.season_loss %> <%= wrestler.criteria %> - <%= wrestler.original_seed %> - <%= wrestler.total_team_points - wrestler.total_points_deducted %> - <%= "Yes" if wrestler.extra? %> - <%= wrestler.next_match_bout_number %> <%= wrestler.next_match_mat_name %> - <% end %> - <% if can? :manage, wrestler.school %> - - <% edit_wrestler_path_with_key = edit_wrestler_path(wrestler) %> - <% edit_wrestler_path_with_key += "?school_permission_key=#{params[:school_permission_key]}" if params[:school_permission_key].present? %> - - <% delete_wrestler_path_with_key = wrestler_path(wrestler) %> - <% delete_wrestler_path_with_key += "?school_permission_key=#{params[:school_permission_key]}" if params[:school_permission_key].present? %> - - <%= link_to edit_wrestler_path_with_key, class: "text-decoration-none" do %> - - <% end %> - <%= link_to delete_wrestler_path_with_key, data: { turbo_method: :delete, turbo_confirm: "Are you sure you want to delete #{wrestler.name}? This will delete all of his matches." }, class: "text-decoration-none" do %> - - <% end %> - - <% end %> - + + <% cache ["school_show_wrestler_cells", wrestler] do %> + <%= render "schools/wrestler_row_cells", wrestler: wrestler %> + <% end %> + <% if can? :manage, wrestler.school %> + + <%= link_to edit_wrestler_path(wrestler), class: "text-decoration-none" do %> + + <% end %> + <%= link_to wrestler_path(wrestler), data: { turbo_method: :delete, turbo_confirm: "Are you sure you want to delete #{wrestler.name}? This will delete all of his matches." }, class: "text-decoration-none" do %> + + <% end %> + + <% end %> + <% end %> <% end %> diff --git a/app/views/tournaments/_bracket_final.html.erb b/app/views/tournaments/_bracket_final.html.erb index 2a22b49..abcdc54 100644 --- a/app/views/tournaments/_bracket_final.html.erb +++ b/app/views/tournaments/_bracket_final.html.erb @@ -1,13 +1,15 @@ <% @final_match.each do |match| %> -
-
-
<%= match.w1_bracket_name.html_safe %>
- <% if params[:print] %> -

<%= match.bout_number %> <%= match.bracket_score_string %>

<%= @winner_place %> Place Winner

- <% else %> -

<%= link_to match.bout_number, spectate_match_path(match) %> <%= match.bracket_score_string %>

<%= @winner_place %> Place Winner

- <% end %> -
<%= match.w2_bracket_name.html_safe %>
-
-
+ <% cache ["bracket_final_match", match, match.wrestler1, match.wrestler2, @winner_place, params[:print].to_s] do %> +
+
+
<%= match.w1_bracket_name.html_safe %>
+ <% if params[:print] %> +

<%= match.bout_number %> <%= match.bracket_score_string %>

<%= @winner_place %> Place Winner

+ <% else %> +

<%= link_to match.bout_number, spectate_match_path(match) %> <%= match.bracket_score_string %>

<%= @winner_place %> Place Winner

+ <% end %> +
<%= match.w2_bracket_name.html_safe %>
+
+
+ <% end %> <% end %> diff --git a/app/views/tournaments/_bracket_round.html.erb b/app/views/tournaments/_bracket_round.html.erb index 8f40f8d..ba5c3a8 100644 --- a/app/views/tournaments/_bracket_round.html.erb +++ b/app/views/tournaments/_bracket_round.html.erb @@ -1,15 +1,17 @@
<% @round_matches.sort_by{|m| m.bracket_position_number}.each do |match| %> -
-
<%= match.w1_bracket_name.html_safe %>
- <% if params[:print] %> -
<%= match.bout_number %> <%= match.bracket_score_string %> 
- <% else %> -
<%= link_to match.bout_number, spectate_match_path(match) %> <%= match.bracket_score_string %> 
- <% end %> -
Round <%= match.round %>
-
<%= match.bracket_position %>
-
<%= match.w2_bracket_name.html_safe %>
-
+ <% cache ["bracket_round_match", match, match.wrestler1, match.wrestler2, params[:print].to_s] do %> +
+
<%= match.w1_bracket_name.html_safe %>
+ <% if params[:print] %> +
<%= match.bout_number %> <%= match.bracket_score_string %> 
+ <% else %> +
<%= link_to match.bout_number, spectate_match_path(match) %> <%= match.bracket_score_string %> 
+ <% end %> +
Round <%= match.round %>
+
<%= match.bracket_position %>
+
<%= match.w2_bracket_name.html_safe %>
+
+ <% end %> <% end %> -
\ No newline at end of file + diff --git a/app/views/tournaments/_team_score_row.html.erb b/app/views/tournaments/_team_score_row.html.erb new file mode 100644 index 0000000..25a34d0 --- /dev/null +++ b/app/views/tournaments/_team_score_row.html.erb @@ -0,0 +1,6 @@ +<% cache ["team_score_row", school, rank] do %> + + <%= rank %>. <%= school.name %> (<%= school.abbreviation %>) + <%= school.page_score_string %> + +<% end %> diff --git a/app/views/tournaments/_up_matches_board.html.erb b/app/views/tournaments/_up_matches_board.html.erb index 523b73d..c80e19b 100644 --- a/app/views/tournaments/_up_matches_board.html.erb +++ b/app/views/tournaments/_up_matches_board.html.erb @@ -31,14 +31,7 @@ - <% (local_assigns[:matches] || tournament.up_matches_unassigned_matches).each do |m| %> - - Round <%= m.round %> - <%= m.bout_number %> - <%= m.weight_max %> - <%= m.w1_bracket_name %> vs. <%= m.w2_bracket_name %> - - <% end %> + <%= render partial: "tournaments/up_matches_unassigned_row", collection: (local_assigns[:matches] || tournament.up_matches_unassigned_matches), as: :match %>
diff --git a/app/views/tournaments/_up_matches_mat_row.html.erb b/app/views/tournaments/_up_matches_mat_row.html.erb index f4095ed..6a24ae9 100644 --- a/app/views/tournaments/_up_matches_mat_row.html.erb +++ b/app/views/tournaments/_up_matches_mat_row.html.erb @@ -1,5 +1,6 @@ <% queue1_match, queue2_match, queue3_match, queue4_match = mat.queue_matches %> -<% cache ["up_matches_mat_row", mat, mat.queue1, mat.queue2, mat.queue3, mat.queue4] do %> +<% queue_match_dependencies = [queue1_match, queue2_match, queue3_match, queue4_match].compact.flat_map { |match| [match, match.wrestler1, match.wrestler2] } %> +<% cache ["up_matches_mat_row", mat, *queue_match_dependencies] do %> <%= mat.name %> diff --git a/app/views/tournaments/_up_matches_unassigned_row.html.erb b/app/views/tournaments/_up_matches_unassigned_row.html.erb new file mode 100644 index 0000000..6c9acc0 --- /dev/null +++ b/app/views/tournaments/_up_matches_unassigned_row.html.erb @@ -0,0 +1,8 @@ +<% cache ["up_matches_unassigned_row", match, match.wrestler1, match.wrestler2] do %> + + Round <%= match.round %> + <%= match.bout_number %> + <%= match.weight_max %> + <%= match.w1_bracket_name %> vs. <%= match.w2_bracket_name %> + +<% end %> diff --git a/app/views/tournaments/bracket.html.erb b/app/views/tournaments/bracket.html.erb index 86fe768..1bfef62 100644 --- a/app/views/tournaments/bracket.html.erb +++ b/app/views/tournaments/bracket.html.erb @@ -1,8 +1,8 @@ -<% cache ["#{@weight.id}_bracket", @weight] do %> +<% cache ["#{@weight.id}_bracket", @weight, params[:print].to_s] do %> <%= render 'bracket_partial' %> <% end %> <% if @tournament.tournament_type == "Pool to bracket" %> <%= render 'pool_bracket_director_actions' %> <% elsif @tournament.tournament_type.include? "Modified 16 Man Double Elimination" or @tournament.tournament_type.include? "Regular Double Elimination" %> <%= render 'bracket_director_actions' %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/tournaments/team_scores.html.erb b/app/views/tournaments/team_scores.html.erb index 60b086b..de083ac 100644 --- a/app/views/tournaments/team_scores.html.erb +++ b/app/views/tournaments/team_scores.html.erb @@ -1,6 +1,5 @@ - -<% cache ["#{@tournament.id}_team_scores", @tournament] do %> - +<% team_scores_last_updated = @schools.map(&:updated_at).compact.max&.utc&.to_fs(:nsec) %> +<% cache ["team_scores", @tournament.id, @schools.size, team_scores_last_updated] do %>

Team Scores

@@ -11,12 +10,9 @@ - <% @schools.each do |school| %> - - - - + <% @schools.each_with_index do |school, index| %> + <%= render "tournaments/team_score_row", school: school, rank: index + 1 %> <% end %>
<%= @schools.index(school) + 1 %>. <%= school.name %> (<%= school.abbreviation %>)<%= school.page_score_string %>
-<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/weights/_readonly_wrestler_row.html.erb b/app/views/weights/_readonly_wrestler_row.html.erb new file mode 100644 index 0000000..95711d9 --- /dev/null +++ b/app/views/weights/_readonly_wrestler_row.html.erb @@ -0,0 +1,10 @@ +<% cache ["weight_show_wrestler_row", wrestler] do %> + + <%= link_to wrestler.name, wrestler %> + <%= wrestler.school.name %> + <%= wrestler.original_seed %> + <%= wrestler.season_win %>-<%= wrestler.season_loss %> + <%= wrestler.criteria %> Win <%= wrestler.season_win_percentage %>% + <%= "Yes" if wrestler.extra? %> + +<% end %> diff --git a/app/views/weights/show.html.erb b/app/views/weights/show.html.erb index b4f0b90..301df13 100644 --- a/app/views/weights/show.html.erb +++ b/app/views/weights/show.html.erb @@ -14,35 +14,36 @@ - <% @wrestlers.sort_by{|w| [w.original_seed ? 0 : 1, w.original_seed || 0]}.each do |wrestler| %> - <% if wrestler.weight_id == @weight.id %> - - <%= link_to "#{wrestler.name}", wrestler %> - <%= wrestler.school.name %> - - <% if can? :manage, @tournament %> - <%= fields_for "wrestler[]", wrestler do |w| %> - <%= w.text_field :original_seed %> - <% end %> - <% else %> - <%= wrestler.original_seed %> - <% end %> - - <%= wrestler.season_win %>-<%= wrestler.season_loss %> - <%= wrestler.criteria %> Win <%= wrestler.season_win_percentage %>% - <% if wrestler.extra? == true %> - Yes - <% end %> - <% if can? :manage, @tournament %> - - <%= link_to wrestler, data: { turbo_method: :delete, turbo_confirm: "Are you sure you want to delete #{wrestler.name}? THIS WILL DELETE ALL MATCHES." }, class: "text-decoration-none" do %> - - <% end %> - - <% end %> - - <% end %> - <% end %> + <% sorted_wrestlers = @wrestlers.sort_by{|w| [w.original_seed ? 0 : 1, w.original_seed || 0]} %> + <% if can? :manage, @tournament %> + <% sorted_wrestlers.each do |wrestler| %> + <% if wrestler.weight_id == @weight.id %> + + <%= link_to wrestler.name, wrestler %> + <%= wrestler.school.name %> + + <%= fields_for "wrestler[]", wrestler do |w| %> + <%= w.text_field :original_seed %> + <% end %> + + <%= wrestler.season_win %>-<%= wrestler.season_loss %> + <%= wrestler.criteria %> Win <%= wrestler.season_win_percentage %>% + <%= "Yes" if wrestler.extra? %> + + <%= link_to wrestler, data: { turbo_method: :delete, turbo_confirm: "Are you sure you want to delete #{wrestler.name}? THIS WILL DELETE ALL MATCHES." }, class: "text-decoration-none" do %> + + <% end %> + + + <% end %> + <% end %> + <% else %> + <% sorted_wrestlers.each do |wrestler| %> + <% if wrestler.weight_id == @weight.id %> + <%= render "weights/readonly_wrestler_row", wrestler: wrestler %> + <% end %> + <% end %> + <% end %>

*All wrestlers without a seed (determined by tournament director) will be assigned a random bracket line.

diff --git a/test/controllers/school_show_cache_test.rb b/test/controllers/school_show_cache_test.rb new file mode 100644 index 0000000..e5ed7b0 --- /dev/null +++ b/test/controllers/school_show_cache_test.rb @@ -0,0 +1,143 @@ +require "test_helper" + +class SchoolShowCacheTest < ActionController::TestCase + tests SchoolsController + + setup do + create_double_elim_tournament_single_weight_1_6(8) + @tournament.update!(user_id: users(:one).id) + @school = @tournament.schools.first + + sign_in users(:one) + + @original_perform_caching = ActionController::Base.perform_caching + ActionController::Base.perform_caching = true + Rails.cache.clear + end + + teardown do + Rails.cache.clear + ActionController::Base.perform_caching = @original_perform_caching + end + + test "school show wrestler cell fragments hit cache and invalidate after wrestler update" do + first_events = cache_events_for_school_show do + get :show, params: { id: @school.id } + assert_response :success + end + assert_operator cache_writes(first_events), :>, 0, "Expected initial school show render to write wrestler cell fragments" + + second_events = cache_events_for_school_show do + get :show, params: { id: @school.id } + assert_response :success + end + assert_equal 0, cache_writes(second_events), "Expected repeat school show render to reuse wrestler cell fragments" + assert_operator cache_hits(second_events), :>, 0, "Expected repeat school show render to hit wrestler cell cache" + + wrestler = @school.wrestlers.first + third_events = cache_events_for_school_show do + wrestler.touch + get :show, params: { id: @school.id } + assert_response :success + end + assert_operator cache_writes(third_events), :>, 0, "Expected wrestler update to invalidate school show wrestler cell cache" + end + + test "school show does not leak manage-only controls from cache across users" do + get :show, params: { id: @school.id } + assert_response :success + assert_includes response.body, "New Wrestler" + assert_match(/fa-trash-alt/, response.body) + assert_match(/fa-edit/, response.body) + + sign_out + + spectator_events = cache_events_for_school_show do + get :show, params: { id: @school.id } + assert_response :success + end + assert_operator cache_hits(spectator_events), :>, 0, "Expected spectator request to hit wrestler cell cache warmed by owner" + assert_not_includes response.body, "New Wrestler" + assert_no_match(/fa-trash-alt/, response.body) + assert_no_match(/fa-edit/, response.body) + end + + test "school show with school_permission_key bypasses cached wrestler cell fragments" do + @school.update!(permission_key: SecureRandom.uuid) + sign_out + + key_request_events = cache_events_for_school_show do + get :show, params: { id: @school.id, school_permission_key: @school.permission_key } + assert_response :success + end + + assert_equal 0, cache_writes(key_request_events), "Expected school_permission_key request to bypass cached wrestler cells" + assert_equal 0, cache_hits(key_request_events), "Expected school_permission_key request to avoid reading cached wrestler cells" + end + + test "completing a match expires school show wrestler cell caches" do + warm_events = cache_events_for_school_show do + get :show, params: { id: @school.id } + assert_response :success + end + assert_operator cache_writes(warm_events), :>, 0, "Expected initial school show render to warm wrestler cell cache" + + wrestler = @school.wrestlers.first + assert wrestler, "Expected a wrestler for match-completion cache test" + match = wrestler.unfinished_matches.first || wrestler.all_matches.first + assert match, "Expected a match involving school wrestler" + + winner_id = match.w1 || match.w2 + assert winner_id, "Expected match to have at least one wrestler slot" + match.update!( + finished: 1, + winner_id: winner_id, + win_type: "Decision", + score: "1-0" + ) + + post_action_events = cache_events_for_school_show do + get :show, params: { id: @school.id } + assert_response :success + end + assert_operator cache_writes(post_action_events), :>, 0, "Expected completed match to expire school show wrestler cell cache" + end + + private + + def sign_out + @request.session[:user_id] = nil + @controller.instance_variable_set(:@current_user, nil) + @controller.instance_variable_set(:@current_ability, nil) + end + + def cache_events_for_school_show + events = [] + subscriber = lambda do |name, _start, _finish, _id, payload| + key = payload[:key].to_s + next unless key.include?("school_show_wrestler_cells") + + events << { name: name, hit: payload[:hit] } + end + + ActiveSupport::Notifications.subscribed( + subscriber, + /cache_(read|write|fetch_hit|generate)\.active_support/ + ) do + yield + end + + events + end + + def cache_writes(events) + events.count { |event| event[:name] == "cache_write.active_support" } + end + + def cache_hits(events) + events.count do |event| + event[:name] == "cache_fetch_hit.active_support" || + (event[:name] == "cache_read.active_support" && event[:hit]) + end + end +end diff --git a/test/controllers/tournament_pages_cache_test.rb b/test/controllers/tournament_pages_cache_test.rb new file mode 100644 index 0000000..365f25f --- /dev/null +++ b/test/controllers/tournament_pages_cache_test.rb @@ -0,0 +1,164 @@ +require "test_helper" + +class TournamentPagesCacheTest < ActionController::TestCase + tests TournamentsController + + setup do + create_double_elim_tournament_single_weight_1_6(8) + @tournament.update!(user_id: users(:one).id) + @weight = @tournament.weights.first + + sign_in users(:one) + + @original_perform_caching = ActionController::Base.perform_caching + ActionController::Base.perform_caching = true + Rails.cache.clear + end + + teardown do + Rails.cache.clear + ActionController::Base.perform_caching = @original_perform_caching + end + + test "team_scores cache hits on repeat render and rewrites after school update" do + first_events = cache_events_for(%w[team_scores team_score_row]) do + get :team_scores, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(first_events), :>, 0, "Expected initial team_scores render to write fragments" + + second_events = cache_events_for(%w[team_scores team_score_row]) do + get :team_scores, params: { id: @tournament.id } + assert_response :success + end + assert_equal 0, cache_writes(second_events), "Expected repeat team_scores render to reuse fragments" + assert_operator cache_hits(second_events), :>, 0, "Expected repeat team_scores render to hit cache" + + school = @tournament.schools.first + third_events = cache_events_for(%w[team_scores team_score_row]) do + school.update!(score: (school.score || 0) + 1) + get :team_scores, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(third_events), :>, 0, "Expected school score update to invalidate team_scores cache" + end + + test "bracket cache hits on repeat render and rewrites after match update" do + key_markers = [@weight.id.to_s + "_bracket", "bracket_round_match", "bracket_final_match"] + + first_events = cache_events_for(key_markers) do + get :bracket, params: { id: @tournament.id, weight: @weight.id } + assert_response :success + end + assert_operator cache_writes(first_events), :>, 0, "Expected initial bracket render to write fragments" + + second_events = cache_events_for(key_markers) do + get :bracket, params: { id: @tournament.id, weight: @weight.id } + assert_response :success + end + assert_equal 0, cache_writes(second_events), "Expected repeat bracket render to reuse fragments" + assert_operator cache_hits(second_events), :>, 0, "Expected repeat bracket render to hit cache" + + match = @weight.matches.first + third_events = cache_events_for(key_markers) do + match.touch + get :bracket, params: { id: @tournament.id, weight: @weight.id } + assert_response :success + end + assert_operator cache_writes(third_events), :>, 0, "Expected match update to invalidate bracket cache" + end + + test "bracket cache separates print and non-print variants" do + key_markers = [@weight.id.to_s + "_bracket"] + + non_print_events = cache_events_for(key_markers) do + get :bracket, params: { id: @tournament.id, weight: @weight.id } + assert_response :success + end + assert_operator cache_writes(non_print_events), :>, 0, "Expected non-print bracket render to write a page fragment" + assert_match(%r{\/matches\/\d+\/spectate}, response.body, "Expected non-print bracket view to include spectate links") + + first_print_events = cache_events_for(key_markers) do + get :bracket, params: { id: @tournament.id, weight: @weight.id, print: true } + assert_response :success + end + assert_operator cache_writes(first_print_events), :>, 0, "Expected first print bracket render to write a separate page fragment" + assert_no_match(%r{\/matches\/\d+\/spectate}, response.body, "Expected print bracket view to omit spectate links") + + second_print_events = cache_events_for(key_markers) do + get :bracket, params: { id: @tournament.id, weight: @weight.id, print: true } + assert_response :success + end + assert_equal 0, cache_writes(second_print_events), "Expected repeat print bracket render to reuse print cache fragment" + assert_operator cache_hits(second_print_events), :>, 0, "Expected repeat print bracket render to hit cache" + end + + test "completing a match expires team_scores and bracket caches" do + team_warm_events = cache_events_for(%w[team_scores team_score_row]) do + get :team_scores, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(team_warm_events), :>, 0, "Expected initial team_scores render to warm cache" + + bracket_key_markers = [@weight.id.to_s + "_bracket", "bracket_round_match", "bracket_final_match"] + bracket_warm_events = cache_events_for(bracket_key_markers) do + get :bracket, params: { id: @tournament.id, weight: @weight.id } + assert_response :success + end + assert_operator cache_writes(bracket_warm_events), :>, 0, "Expected initial bracket render to warm cache" + + match = @weight.matches.where(finished: [nil, 0]).first || @weight.matches.first + assert match, "Expected a match to complete for expiration test" + + match.update!( + finished: 1, + winner_id: match.w1 || match.w2, + win_type: "Decision", + score: "1-0" + ) + + team_post_events = cache_events_for(%w[team_scores team_score_row]) do + get :team_scores, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(team_post_events), :>, 0, "Expected completed match to expire team_scores cache" + + bracket_post_events = cache_events_for(bracket_key_markers) do + get :bracket, params: { id: @tournament.id, weight: @weight.id } + assert_response :success + end + assert_operator cache_writes(bracket_post_events), :>, 0, "Expected completed match to expire bracket cache" + end + + private + + def cache_events_for(key_markers) + events = [] + subscriber = lambda do |name, _start, _finish, _id, payload| + key = payload[:key].to_s + next unless key_markers.any? { |marker| key.include?(marker) } + + events << { name: name, hit: payload[:hit] } + end + + ActiveSupport::Notifications.subscribed( + subscriber, + /cache_(read|write|fetch_hit|generate)\.active_support/ + ) do + yield + end + + events + end + + def cache_writes(events) + events.count { |event| event[:name] == "cache_write.active_support" } + end + + def cache_hits(events) + events.count do |event| + event[:name] == "cache_fetch_hit.active_support" || + (event[:name] == "cache_read.active_support" && event[:hit]) + end + end +end diff --git a/test/controllers/up_matches_cache_test.rb b/test/controllers/up_matches_cache_test.rb index f48d341..27d45d1 100644 --- a/test/controllers/up_matches_cache_test.rb +++ b/test/controllers/up_matches_cache_test.rb @@ -73,13 +73,86 @@ class UpMatchesCacheTest < ActionController::TestCase assert_operator cache_hits(repeat_events), :>, 0, "Expected cache hits after queue clear rewrite" end + test "up_matches unassigned row fragments hit cache and invalidate after unassigned match update" do + key_markers = %w[up_matches_unassigned_row] + + first_events = cache_events_for_up_matches(key_markers) do + get :up_matches, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(first_events), :>, 0, "Expected initial unassigned row render to write fragments" + + second_events = cache_events_for_up_matches(key_markers) do + get :up_matches, params: { id: @tournament.id } + assert_response :success + end + assert_equal 0, cache_writes(second_events), "Expected repeat unassigned row render to reuse cached fragments" + assert_operator cache_hits(second_events), :>, 0, "Expected repeat unassigned row render to hit cache" + + unassigned_match = @tournament.up_matches_unassigned_matches.first + assert unassigned_match, "Expected at least one unassigned match for cache invalidation test" + + third_events = cache_events_for_up_matches(key_markers) do + unassigned_match.touch + get :up_matches, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(third_events), :>, 0, "Expected unassigned match update to invalidate unassigned row fragment" + end + + test "completing an on-mat match expires up_matches cached fragments" do + warm_events = cache_events_for_up_matches(%w[up_matches_mat_row up_matches_unassigned_row]) do + get :up_matches, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(warm_events), :>, 0, "Expected initial up_matches render to warm caches" + + mat = @tournament.mats.detect { |m| m.queue1_match.present? } + assert mat, "Expected a mat with a queued match" + match = mat.queue1_match + assert match, "Expected queue1 match to complete" + + post_action_events = cache_events_for_up_matches(%w[up_matches_mat_row up_matches_unassigned_row]) do + match.update!( + finished: 1, + winner_id: match.w1 || match.w2, + win_type: "Decision", + score: "1-0" + ) + get :up_matches, params: { id: @tournament.id } + assert_response :success + end + + assert_operator cache_writes(post_action_events), :>, 0, "Expected completed match to expire and rewrite up_matches caches" + end + + test "manually assigning an unassigned match to a mat queue expires up_matches caches" do + warm_events = cache_events_for_up_matches(%w[up_matches_mat_row up_matches_unassigned_row]) do + get :up_matches, params: { id: @tournament.id } + assert_response :success + end + assert_operator cache_writes(warm_events), :>, 0, "Expected initial up_matches render to warm caches" + + unassigned_match = @tournament.up_matches_unassigned_matches.first + assert unassigned_match, "Expected at least one unassigned match to manually place on a mat" + target_mat = @tournament.mats.first + + post_action_events = cache_events_for_up_matches(%w[up_matches_mat_row up_matches_unassigned_row]) do + target_mat.assign_match_to_queue!(unassigned_match, 4) + get :up_matches, params: { id: @tournament.id } + assert_response :success + end + + assert_operator cache_writes(post_action_events), :>, 0, "Expected manual mat assignment to expire and rewrite up_matches caches" + end + private - def cache_events_for_up_matches + def cache_events_for_up_matches(key_markers = %w[up_matches_mat_row up_matches_unassigned_row]) events = [] subscriber = lambda do |name, _start, _finish, _id, payload| key = payload[:key].to_s - next unless key.include?("up_matches_mat_row") + next unless key_markers.any? { |marker| key.include?(marker) } events << { name: name, hit: payload[:hit] } end diff --git a/test/controllers/weight_show_cache_test.rb b/test/controllers/weight_show_cache_test.rb new file mode 100644 index 0000000..0a2640a --- /dev/null +++ b/test/controllers/weight_show_cache_test.rb @@ -0,0 +1,107 @@ +require "test_helper" + +class WeightShowCacheTest < ActionController::TestCase + tests WeightsController + + setup do + create_a_tournament_with_single_weight("Regular Double Elimination 1-6", 8) + @tournament.update!(user_id: users(:one).id) + @weight = @tournament.weights.first + + @original_perform_caching = ActionController::Base.perform_caching + ActionController::Base.perform_caching = true + Rails.cache.clear + end + + teardown do + Rails.cache.clear + ActionController::Base.perform_caching = @original_perform_caching + end + + test "weight show readonly row fragments hit cache and invalidate after wrestler update" do + first_events = cache_events_for_weight_show do + get :show, params: { id: @weight.id } + assert_response :success + end + assert_operator cache_writes(first_events), :>, 0, "Expected initial weight show render to write readonly row fragments" + + second_events = cache_events_for_weight_show do + get :show, params: { id: @weight.id } + assert_response :success + end + assert_equal 0, cache_writes(second_events), "Expected repeat weight show render to reuse readonly row fragments" + assert_operator cache_hits(second_events), :>, 0, "Expected repeat weight show render to hit readonly row cache" + + wrestler = @weight.wrestlers.first + third_events = cache_events_for_weight_show do + wrestler.touch + get :show, params: { id: @weight.id } + assert_response :success + end + assert_operator cache_writes(third_events), :>, 0, "Expected wrestler update to invalidate weight show readonly row cache" + end + + test "weight show does not leak manage-only controls from cache across users" do + sign_in users(:one) + get :show, params: { id: @weight.id } + assert_response :success + assert_includes response.body, "Save Seeds" + assert_match(/fa-trash-alt/, response.body) + assert_match(/name="wrestler\[\d+\]\[original_seed\]"/, response.body) + + sign_out + + get :show, params: { id: @weight.id } + assert_response :success + assert_not_includes response.body, "Save Seeds" + assert_no_match(/fa-trash-alt/, response.body) + assert_no_match(/name="wrestler\[\d+\]\[original_seed\]"/, response.body) + + spectator_cache_events = cache_events_for_weight_show do + get :show, params: { id: @weight.id } + assert_response :success + end + assert_operator cache_hits(spectator_cache_events), :>, 0, "Expected repeat spectator request to hit readonly wrestler row cache" + assert_not_includes response.body, "Save Seeds" + assert_no_match(/fa-trash-alt/, response.body) + assert_no_match(/name="wrestler\[\d+\]\[original_seed\]"/, response.body) + end + + private + + def sign_out + @request.session[:user_id] = nil + @controller.instance_variable_set(:@current_user, nil) + @controller.instance_variable_set(:@current_ability, nil) + end + + def cache_events_for_weight_show + events = [] + subscriber = lambda do |name, _start, _finish, _id, payload| + key = payload[:key].to_s + next unless key.include?("weight_show_wrestler_row") + + events << { name: name, hit: payload[:hit] } + end + + ActiveSupport::Notifications.subscribed( + subscriber, + /cache_(read|write|fetch_hit|generate)\.active_support/ + ) do + yield + end + + events + end + + def cache_writes(events) + events.count { |event| event[:name] == "cache_write.active_support" } + end + + def cache_hits(events) + events.count do |event| + event[:name] == "cache_fetch_hit.active_support" || + (event[:name] == "cache_read.active_support" && event[:hit]) + end + end +end