mirror of
https://github.com/basecamp/once-campfire.git
synced 2026-10-09 08:10:08 +09:00
Reuse epoch-verified record snapshots and avoid empty push payloads
This commit is contained in:
@@ -1,7 +1,12 @@
|
||||
module Authentication::SessionLookup
|
||||
def find_session_by_cookie
|
||||
if token = cookies.signed[:session_token]
|
||||
Session.find_by(token: token)
|
||||
version = @response_cache_version if @response_cache_version && (request.get? || request.head?)
|
||||
session, user = RecordCache.fetch([ "session", Digest::SHA256.hexdigest(token.to_s) ], version) do
|
||||
session = Session.find_by(token: token)
|
||||
[ session, session&.user ]
|
||||
end
|
||||
session&.tap { |record| record.user = user }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -35,6 +35,10 @@ module CachedResponses
|
||||
end
|
||||
|
||||
private
|
||||
def read_record_cache_version
|
||||
@response_cache_version if request.get? || request.head?
|
||||
end
|
||||
|
||||
def capture_response_cache_version
|
||||
# Capture for native HTML/JSON/stream renders too, even with page reuse off.
|
||||
# Detached renderers do not run callbacks and therefore render uncached.
|
||||
@@ -81,6 +85,10 @@ module CachedResponses
|
||||
def cache_completed_response(key, original_session, encoding)
|
||||
if response.status == 200 && response.media_type == "text/html" && session.to_hash == original_session && !response.headers["Content-Encoding"]
|
||||
body = encoding == "gzip" ? Zlib.gzip(response.body) : response.body
|
||||
unless body.empty? || response.headers["ETag"] || response.headers["Last-Modified"]
|
||||
# Match Rack::ETag once, rather than hashing the same bytes on each hit.
|
||||
response.headers["ETag"] = %(W/"#{Digest::SHA256.hexdigest(body).byteslice(0, 32)}")
|
||||
end
|
||||
response.headers["Content-Encoding"] = "gzip" if encoding == "gzip"
|
||||
response.headers["Vary"] = (response.headers["Vary"].to_s.split(/,\s*/) | [ "Accept-Encoding" ]).join(", ")
|
||||
headers = response.headers.slice(*CACHE_HEADERS).to_h.freeze
|
||||
|
||||
@@ -7,7 +7,10 @@ module RoomScoped
|
||||
|
||||
private
|
||||
def set_room
|
||||
@membership = Current.user.memberships.find_by!(room_id: params[:room_id])
|
||||
@room = @membership.room
|
||||
@membership, @room = RecordCache.fetch([ "membership", Current.user.id, params[:room_id] ], read_record_cache_version) do
|
||||
membership = Current.user.memberships.find_by!(room_id: params[:room_id])
|
||||
[ membership, membership.room ]
|
||||
end
|
||||
@membership.room = @room
|
||||
end
|
||||
end
|
||||
|
||||
@@ -21,7 +21,10 @@ class RoomsController < ApplicationController
|
||||
|
||||
private
|
||||
def set_room
|
||||
if room = room_scope.find_by(id: params[:room_id] || params[:id])
|
||||
room = RecordCache.fetch([ "room", self.class.name, Current.user.id, params[:room_id] || params[:id] ], read_record_cache_version) do
|
||||
[ room_scope.find_by(id: params[:room_id] || params[:id]) ]
|
||||
end.first
|
||||
if room
|
||||
@room = room
|
||||
else
|
||||
redirect_to root_url, alert: "Room not found or inaccessible"
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
# Rebuild request-local models from bounded, immutable database snapshots.
|
||||
# The observer must still see the captured epoch after lookup and admission.
|
||||
class RecordCache
|
||||
def self.fetch(key, version)
|
||||
return yield unless version && ResponseCache.instance.budget.positive? && !ActiveRecord::Base.connection.transaction_open?
|
||||
|
||||
key = ActiveSupport::Cache.expand_cache_key([ "record-snapshot-v1", version, key ])
|
||||
return yield if key.bytesize > ResponseCache::MAX_KEY_BYTES
|
||||
|
||||
if snapshot = FragmentCache.store.read(key)
|
||||
if ResponseCache.instance.version == version
|
||||
return ActiveSupport::JSON.decode(snapshot).map { |name, attributes| name.constantize.instantiate(attributes) }
|
||||
end
|
||||
end
|
||||
|
||||
records = yield
|
||||
if records.all? { |record| record&.persisted? && !record.changed? } && ResponseCache.instance.version == version
|
||||
# Raw database values retain timestamp precision and native enum binding.
|
||||
snapshot = ActiveSupport::JSON.encode(records.map { |record| [ record.class.name, record.attributes_before_type_cast ] })
|
||||
FragmentCache.store.write(key, snapshot) if snapshot.bytesize <= ResponseCache::MAX_ENTRY_BYTES
|
||||
end
|
||||
records
|
||||
end
|
||||
end
|
||||
@@ -6,9 +6,9 @@ class Room::MessagePusher
|
||||
end
|
||||
|
||||
def push
|
||||
build_payload.tap do |payload|
|
||||
push_to_users_involved_in_everything(payload)
|
||||
push_to_users_involved_in_mentions(payload)
|
||||
subscriptions = push_subscriptions_for_users_involved_in_everything.or(push_subscriptions_for_mentionable_users(message.mentionees))
|
||||
if subscriptions.exists?
|
||||
Rails.configuration.x.web_push_pool.queue(build_payload, subscriptions)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -37,14 +37,6 @@ class Room::MessagePusher
|
||||
}
|
||||
end
|
||||
|
||||
def push_to_users_involved_in_everything(payload)
|
||||
enqueue_payload_for_delivery payload, push_subscriptions_for_users_involved_in_everything
|
||||
end
|
||||
|
||||
def push_to_users_involved_in_mentions(payload)
|
||||
enqueue_payload_for_delivery payload, push_subscriptions_for_mentionable_users(message.mentionees)
|
||||
end
|
||||
|
||||
def push_subscriptions_for_users_involved_in_everything
|
||||
relevant_subscriptions.merge(Membership.involved_in_everything)
|
||||
end
|
||||
@@ -59,10 +51,6 @@ class Room::MessagePusher
|
||||
Push::Subscription
|
||||
.joins(user: :memberships)
|
||||
.merge(User.active)
|
||||
.merge(Membership.visible.disconnected.where(room: room).where.not(user: message.creator))
|
||||
end
|
||||
|
||||
def enqueue_payload_for_delivery(payload, subscriptions)
|
||||
Rails.configuration.x.web_push_pool.queue(payload, subscriptions)
|
||||
.merge(Membership.visible.disconnected.where(room: room).where.not(user_id: message.creator_id))
|
||||
end
|
||||
end
|
||||
|
||||
@@ -11,8 +11,9 @@
|
||||
|
||||
<div class="message__actions-menu border shadow" data-popup-target="menu">
|
||||
<div class="quick-boosts">
|
||||
<% boost_path = message_boosts_path(message) %>
|
||||
<% EmojiHelper::REACTIONS.each do |character, title| %>
|
||||
<%= form_with model: [ message, Boost.new ], data: { turbo_frame: dom_id(message, :boosting), action: "popup#close"} do |form| %>
|
||||
<%= form_with url: boost_path, data: { turbo_frame: dom_id(message, :boosting), action: "popup#close"} do |form| %>
|
||||
<%= hidden_field_tag "boost[content]", character %>
|
||||
<%= form.button type: "submit", title: title, class: "btn message__action-btn", data: { emoji: character } do %>
|
||||
<figure class="margin-none boost-character"><%= character %></figure>
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
require "test_helper"
|
||||
require "tmpdir"
|
||||
|
||||
class RecordCacheTest < ActiveSupport::TestCase
|
||||
setup do
|
||||
@directory = Dir.mktmpdir
|
||||
@database = File.join(@directory, "records.sqlite3")
|
||||
SQLite3::Database.new(@database) do |database|
|
||||
database.execute("PRAGMA journal_mode=WAL")
|
||||
database.execute("CREATE TABLE changes_for_test (value TEXT)")
|
||||
end
|
||||
ActiveRecord::Base.stubs(:connection_db_config).returns(Struct.new(:database).new(@database))
|
||||
ActiveRecord::Base.connection.stubs(:transaction_open?).returns(false)
|
||||
@cache = ResponseCache.new
|
||||
@cache.stubs(:budget).returns(4096)
|
||||
ResponseCache.stubs(:instance).returns(@cache)
|
||||
FragmentCache.stubs(:store).returns(ActiveSupport::Cache::MemoryStore.new(size: 64.kilobytes))
|
||||
@record = User.instantiate(users(:david).attributes_before_type_cast.merge(
|
||||
"created_at" => "2026-10-08 12:34:56.123456", "updated_at" => "2026-10-08 12:34:57.654321",
|
||||
"status" => 2, "role" => 1))
|
||||
end
|
||||
|
||||
teardown do
|
||||
@cache.clear
|
||||
FileUtils.remove_entry(@directory)
|
||||
end
|
||||
|
||||
test "hits reconstruct independent persisted models without losing raw timestamps or enum values" do
|
||||
version = @cache.version
|
||||
assert_same @record, RecordCache.fetch("user", version) { [ @record ] }.first
|
||||
first = RecordCache.fetch("user", version) { flunk "cache hit queried records" }.first
|
||||
second = RecordCache.fetch("user", version) { flunk "cache hit queried records" }.first
|
||||
|
||||
assert_not_same @record, first
|
||||
assert_not_same first, second
|
||||
assert first.persisted?
|
||||
assert_not first.changed?
|
||||
assert_equal @record.attributes_before_type_cast, first.attributes_before_type_cast
|
||||
assert_equal 123456, first.created_at.usec
|
||||
assert_equal 654321, first.updated_at.usec
|
||||
assert_equal "banned", first.status
|
||||
assert_equal "administrator", first.role
|
||||
first.name = "Request-local edit"
|
||||
assert_equal @record.name, second.name
|
||||
assert_equal @record.name, RecordCache.fetch("user", version) { flunk "cache hit queried records" }.first.name
|
||||
end
|
||||
|
||||
test "foreign commit makes an old captured epoch query fresh records instead of reusing a snapshot" do
|
||||
version = @cache.version
|
||||
RecordCache.fetch("user", version) { [ @record ] }
|
||||
foreign_commit
|
||||
fresh = User.instantiate(@record.attributes_before_type_cast.merge("name" => "Changed elsewhere"))
|
||||
assert_same fresh, RecordCache.fetch("user", version) { [ fresh ] }.first
|
||||
assert_same fresh, RecordCache.fetch("user", @cache.version) { [ fresh ] }.first
|
||||
assert_equal fresh.name, RecordCache.fetch("user", @cache.version) { flunk "fresh snapshot missing" }.first.name
|
||||
end
|
||||
|
||||
test "observer is rechecked after a snapshot lookup" do
|
||||
version = @cache.version
|
||||
RecordCache.fetch("user", version) { [ @record ] }
|
||||
store = FragmentCache.store
|
||||
original_read = store.method(:read)
|
||||
commit = method(:foreign_commit)
|
||||
store.define_singleton_method(:read) do |*arguments|
|
||||
original_read.call(*arguments).tap { |snapshot| commit.call if snapshot }
|
||||
end
|
||||
fresh = User.instantiate(@record.attributes_before_type_cast.merge("name" => "Committed during lookup"))
|
||||
assert_same fresh, RecordCache.fetch("user", version) { [ fresh ] }.first
|
||||
end
|
||||
|
||||
test "a commit while loading records prevents admission under the old epoch" do
|
||||
version = @cache.version
|
||||
FragmentCache.store.expects(:write).never
|
||||
assert_same @record, RecordCache.fetch("user", version) { foreign_commit; [ @record ] }.first
|
||||
replacement = User.instantiate(@record.attributes_before_type_cast.merge("name" => "Fresh load"))
|
||||
assert_same replacement, RecordCache.fetch("user", version) { [ replacement ] }.first
|
||||
end
|
||||
|
||||
test "disabled budget missing epoch and open transactions always query and never admit" do
|
||||
version = @cache.version
|
||||
RecordCache.fetch("user", version) { [ @record ] }
|
||||
FragmentCache.store.expects(:write).never
|
||||
@cache.stubs(:budget).returns(0)
|
||||
assert_same @record, RecordCache.fetch("user", version) { [ @record ] }.first
|
||||
@cache.stubs(:budget).returns(4096)
|
||||
assert_same @record, RecordCache.fetch("user", nil) { [ @record ] }.first
|
||||
ActiveRecord::Base.connection.stubs(:transaction_open?).returns(true)
|
||||
assert_same @record, RecordCache.fetch("user", version) { [ @record ] }.first
|
||||
assert_same @record, RecordCache.fetch("transaction-only", version) { [ @record ] }.first
|
||||
end
|
||||
|
||||
private
|
||||
def foreign_commit
|
||||
SQLite3::Database.new(@database) { |database| database.execute("INSERT INTO changes_for_test VALUES ('committed')") }
|
||||
end
|
||||
end
|
||||
@@ -89,6 +89,16 @@ class Room::PushTest < ActiveSupport::TestCase
|
||||
assert_includes pushed_users { post_to_designers_mentioning_kevin }, users(:kevin)
|
||||
end
|
||||
|
||||
test "does not build notification payloads without recipients" do
|
||||
Push::Subscription.delete_all
|
||||
message = messages(:first)
|
||||
message.expects(:plain_text_body).never
|
||||
message.expects(:creator).never
|
||||
Rails.configuration.x.web_push_pool.expects(:queue).never
|
||||
|
||||
Room::MessagePusher.new(room: message.room, message: message).push
|
||||
end
|
||||
|
||||
private
|
||||
def post_to_designers_mentioning_kevin
|
||||
rooms(:designers).messages.create! body: "Hey #{mention_attachment_for(:kevin)}", client_message_id: "earth", creator: users(:david)
|
||||
|
||||
Reference in New Issue
Block a user