diff --git a/app/controllers/concerns/authentication/session_lookup.rb b/app/controllers/concerns/authentication/session_lookup.rb index f9c0181..b180f21 100644 --- a/app/controllers/concerns/authentication/session_lookup.rb +++ b/app/controllers/concerns/authentication/session_lookup.rb @@ -1,7 +1,7 @@ module Authentication::SessionLookup def find_session_by_cookie if token = cookies.signed[:session_token] - Session.eager_load(:user).find_by(token: token) + Session.find_by(token: token) end end end diff --git a/app/controllers/concerns/cached_responses.rb b/app/controllers/concerns/cached_responses.rb index bc45c88..e2458b7 100644 --- a/app/controllers/concerns/cached_responses.rb +++ b/app/controllers/concerns/cached_responses.rb @@ -26,11 +26,12 @@ module CachedResponses end def combined_fragment_cache_key(key) - @fragment_cache_namespace ||= [ - @response_cache_version, request.base_url, request.script_name, request.format.to_s, I18n.locale, + @fragment_cache_context ||= [ + request.base_url, request.script_name, request.format.to_s, I18n.locale, Current.user&.id, (Digest::SHA256.hexdigest(Current.session.token) if Current.session) ].freeze - super([ @fragment_cache_namespace, key ]) + version = @response_cache_version unless Array(key).flatten.any? { |part| part.is_a?(FragmentCache::ContentKey) } + super([ version, @fragment_cache_context, key ]) end private diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb index 6ef8590..fcc4dcf 100644 --- a/app/helpers/messages_helper.rb +++ b/app/helpers/messages_helper.rb @@ -83,6 +83,22 @@ module MessagesHelper FragmentCache.store.fetch(key) { capture(&block) } end + def message_fragment_cache_key(message) + return message unless controller.perform_caching && + %i[room creator rich_text_body boosts attachment_attachment].all? { |name| message.association(name).loaded? } + + room = message.room + body = message.body.body + return message if room.direct? || message.attachment? || !body || body.to_html.include?(" do diff --git a/app/models/fragment_cache.rb b/app/models/fragment_cache.rb index 63d7702..bb0f7e9 100644 --- a/app/models/fragment_cache.rb +++ b/app/models/fragment_cache.rb @@ -1,5 +1,12 @@ # Native view caches share one byte budget across every database generation. class FragmentCache + # Only callers that fingerprint every rendered dependency may omit the epoch. + ContentKey = Data.define(:digest) do + def cache_key + digest + end + end + STORE = ActiveSupport::Cache::MemoryStore.new(size: 64.megabytes) private_constant :STORE diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb index 0e02bc4..307e31f 100644 --- a/app/views/messages/_message.html.erb +++ b/app/views/messages/_message.html.erb @@ -1,7 +1,7 @@ <%# Be sure to check/update messages/_template.html.erb when changing this file %> <%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %> -<% cache [ message, "presentation-v6" ] do %> +<% cache [ message_fragment_cache_key(message), "presentation-v7" ] do %> <%= message_tag message do %>

<%= local_datetime_tag message.created_at, style: :date %>

diff --git a/app/views/messages/index.html.erb b/app/views/messages/index.html.erb index 6ee288f..6ec11a3 100644 --- a/app/views/messages/index.html.erb +++ b/app/views/messages/index.html.erb @@ -1 +1 @@ -<%= render partial: "messages/message", collection: @messages, cached: true %> +<%= render partial: "messages/message", collection: @messages, cached: ->(message) { message_fragment_cache_key(message) } %> diff --git a/app/views/rooms/refreshes/show.turbo_stream.erb b/app/views/rooms/refreshes/show.turbo_stream.erb index 9afa1d8..d2d485f 100644 --- a/app/views/rooms/refreshes/show.turbo_stream.erb +++ b/app/views/rooms/refreshes/show.turbo_stream.erb @@ -1,5 +1,5 @@ <%= turbo_stream.append dom_id(@room, :messages) do %> - <%= render partial: "messages/message", collection: @new_messages, cached: true %> + <%= render partial: "messages/message", collection: @new_messages, cached: ->(message) { message_fragment_cache_key(message) } %> <% end if @new_messages.any? %> <% @updated_messages.each do |message| %> diff --git a/app/views/rooms/show.html.erb b/app/views/rooms/show.html.erb index ef76ecd..234aac0 100644 --- a/app/views/rooms/show.html.erb +++ b/app/views/rooms/show.html.erb @@ -14,7 +14,7 @@ <%= messages_tag(@room) do %> <%= render "rooms/show/invitation", room: @room %> - <%= render partial: "messages/message", collection: @messages, cached: true %> + <%= render partial: "messages/message", collection: @messages, cached: ->(message) { message_fragment_cache_key(message) } %> <% end %> <%= turbo_stream_from @room, :messages, channel: "RoomMessagesChannel" %> diff --git a/app/views/searches/index.html.erb b/app/views/searches/index.html.erb index cbaa476..03aff66 100644 --- a/app/views/searches/index.html.erb +++ b/app/views/searches/index.html.erb @@ -54,7 +54,7 @@ <%= search_results_tag do %> - <%= render partial: "messages/message", collection: @messages, cached: true %> + <%= render partial: "messages/message", collection: @messages, cached: ->(message) { message_fragment_cache_key(message) } %> <% end %> diff --git a/test/controllers/fragment_cache_test.rb b/test/controllers/fragment_cache_test.rb index f74953a..484bbf2 100644 --- a/test/controllers/fragment_cache_test.rb +++ b/test/controllers/fragment_cache_test.rb @@ -74,6 +74,21 @@ class FragmentRenderingTest < ActionDispatch::IntegrationTest assert_select "form[action=?]", message_boosts_path(@message) end + test "a commit in another room retains content-validated message collection hits" do + ResponseCache.instance.stubs(:budget).returns(0) + get room_messages_url(@room) + assert_response :success + foreign_write("UPDATE rooms SET name = ? WHERE id = ?", "Unrelated room rename", rooms(:pets).id) + collections = [] + ActiveSupport::Notifications.subscribed(->(event) { collections << event.payload }, "render_collection.action_view") do + get room_messages_url(@room) + end + assert_response :success + messages = collections.find { |payload| payload[:identifier].end_with?("messages/_message.html.erb") } + assert messages + assert_equal @room.messages.count, messages[:cache_hits] + end + test "a foreign commit after capture bypasses old native fragment lookups" do ResponseCache.instance.stubs(:budget).returns(0) get room_messages_url(@room)