Reuse token-free message controls and render posting deliveries once

This commit is contained in:
GPT on behalf of DHH
2026-10-08 11:43:27 +02:00
parent e55b6dc3e5
commit 2c53c46767
11 changed files with 66 additions and 10 deletions
@@ -1,7 +1,7 @@
module Authentication::SessionLookup
def find_session_by_cookie
if token = cookies.signed[:session_token]
Session.find_by(token: token)
Session.eager_load(:user).find_by(token: token)
end
end
end
+1 -2
View File
@@ -47,8 +47,6 @@ module CachedResponses
return yield unless encoding
key = response_cache_key(encoding)
original_session = session.to_hash.deep_dup
return yield if key.bytesize > ResponseCache::MAX_KEY_BYTES
entry = ResponseCache.instance.read(key, @response_cache_version)
@@ -57,6 +55,7 @@ module CachedResponses
ResponseCache.instance.synchronize_render(key, @response_cache_version) do
entry = ResponseCache.instance.read(key, @response_cache_version)
if !entry && ResponseCache.instance.version == @response_cache_version
original_session = session.to_hash.deep_dup
yield
rendered = true
entry = cache_completed_response(key, original_session, encoding)
+3 -1
View File
@@ -28,7 +28,9 @@ class MessagesController < ApplicationController
set_room
@message = @room.messages.create_with_attachment!(message_params)
@message.broadcast_create
# Both deliveries contain the same token-free, viewer-independent markup.
@message_html = render_to_string partial: "messages/message", formats: :html, locals: { message: @message }
@message.broadcast_create(html: @message_html)
deliver_webhooks_to_bots
rescue ActiveRecord::RecordNotFound
render action: :room_not_found
+28 -2
View File
@@ -63,8 +63,7 @@ module MessagesHelper
when "sound"
message_sound_presentation(message)
else
auto_link h(ContentFilters::TextMessagePresentationFilters.apply(message.body.body)),
html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES }
text_message_presentation(message.body.body)
end
rescue Exception => e
Sentry.capture_exception(e, extra: { message: message })
@@ -73,7 +72,34 @@ module MessagesHelper
""
end
# These controls contain URLs and static markup, but no viewer or token state.
# Keep them across database commits; attachment controls still render afresh.
def cache_message_actions(message, &block)
return capture(&block) unless controller.perform_caching && !message.attachment?
key = fragment_name_with_digest([
"message-actions-v1", request.base_url, request.script_name, I18n.locale, message.id, message.room_id
], nil)
FragmentCache.store.fetch(key) { capture(&block) }
end
private
def text_message_presentation(body)
render = -> do
auto_link h(ContentFilters::TextMessagePresentationFilters.apply(body)),
html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES }
end
# Embedded attachments render database-backed metadata and signed URLs.
# Plain text HTML depends only on its content, even after a foreign edit.
html = body.to_html
if controller.perform_caching && !html.include?("<action-text-attachment") && html.bytesize <= ResponseCache::MAX_ENTRY_BYTES
FragmentCache.store.fetch([ "text-presentation-v1", I18n.locale, Digest::SHA256.hexdigest(html) ]) { render.call }
else
render.call
end
end
def messages_actions
"turbo:before-stream-render@document->messages#beforeStreamRender keydown.up@document->messages#editMyLastMessage"
end
+2 -2
View File
@@ -1,6 +1,6 @@
module Message::Broadcasts
def broadcast_create
broadcast_append_to room, :messages, target: [ room, :messages ]
def broadcast_create(html: nil)
broadcast_append_to room, :messages, target: [ room, :messages ], **(html ? { html: html } : {})
broadcast_unread_room_later
end
+2
View File
@@ -1,5 +1,6 @@
<%# Be sure to check/update messages/_template.html.erb when changing this file %>
<%= cache_message_actions(message) do %>
<div class="message__actions" data-controller="soft-keyboard">
<%= tag.details class: "position-relative",
data: { controller: "popup", action: "keydown.esc->popup#close toggle->popup#toggle click@document->popup#closeOnClickOutside", popup_orientation_top_class: "popup-orientation-top" } do %>
@@ -56,3 +57,4 @@
</div>
<% end %>
</div>
<% end %>
+1 -1
View File
@@ -1 +1 @@
<%= turbo_stream.append dom_id(@message.room, :messages), @message %>
<%= turbo_stream.append dom_id(@message.room, :messages), @message_html %>
+11
View File
@@ -63,6 +63,17 @@ class FragmentRenderingTest < ActionDispatch::IntegrationTest
assert_includes response.body, "foreign conditional body"
end
test "message controls survive creator edits while the author is rendered fresh" do
get room_messages_url(@room)
assert_response :success
foreign_write("UPDATE users SET name = ? WHERE id = ?", "Fresh creator with cached controls", @message.creator_id)
ActionView::Base.any_instance.expects(:form_with).never
get room_messages_url(@room)
assert_response :success
assert_includes response.body, "Fresh creator with cached controls"
assert_select "form[action=?]", message_boosts_path(@message)
end
test "a foreign commit after capture bypasses old native fragment lookups" do
ResponseCache.instance.stubs(:budget).returns(0)
get room_messages_url(@room)
@@ -49,8 +49,12 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest
end
test "creating a message broadcasts the message to the room" do
ContentFilters::TextMessagePresentationFilters.expects(:apply).once.with { |body| body.to_plain_text == "New one" }.returns(ActionText::Content.new("New one"))
post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } }
assert_response :success
assert_select "turbo-stream[action=append] .message__body", text: /New one/
assert_rendered_turbo_stream_broadcast @room, :messages, action: "append", target: [ @room, :messages ] do
assert_select ".message__body", text: /New one/
assert_copy_link_button room_at_message_url(@room, Message.last, host: "once.campfire.test")
+1 -1
View File
@@ -172,7 +172,7 @@ class ContentFiltersTest < ActionView::TestCase
filtered = ContentFilters::TextMessagePresentationFilters.apply(message.body.body).to_html
assert_equal body, filtered
assert_dom_equal body, filtered
assert_match %r{<table>.*<th><p>Name</p></th>.*<td><p>Jason</p></td>}m, message_presentation(message)
end
+12
View File
@@ -1,6 +1,18 @@
require "test_helper"
class MessagesHelperTest < ActionView::TestCase
test "plain text presentation is reused by content rather than database epoch" do
view.controller.stubs(:perform_caching).returns(true)
FragmentCache.store.clear
message = Message.create! room: rooms(:pets), body: "A reusable <strong>safe</strong> body", creator: users(:jason)
first = view.message_presentation(message)
assert_includes first, "A reusable <strong>safe</strong> body"
ContentFilters::TextMessagePresentationFilters.expects(:apply).never
assert_equal first, view.message_presentation(message)
ensure
FragmentCache.store.clear
end
test "message_presentation neutralizes unsafe URI schemes in links" do
message = Message.create! room: rooms(:pets), body: '<div><a href="javascript:alert(1)">x</a></div>', client_message_id: "0015", creator: users(:jason)