diff --git a/app/jobs/message/broadcast_unread_room_job.rb b/app/jobs/message/broadcast_unread_room_job.rb new file mode 100644 index 0000000..4b0bc2c --- /dev/null +++ b/app/jobs/message/broadcast_unread_room_job.rb @@ -0,0 +1,5 @@ +class Message::BroadcastUnreadRoomJob < ApplicationJob + def perform(message) + message.broadcast_unread_room + end +end diff --git a/app/models/message/broadcasts.rb b/app/models/message/broadcasts.rb index f82e4e3..b9660c9 100644 --- a/app/models/message/broadcasts.rb +++ b/app/models/message/broadcasts.rb @@ -1,21 +1,27 @@ module Message::Broadcasts def broadcast_create broadcast_append_to room, :messages, target: [ room, :messages ] - broadcast_unread_room + broadcast_unread_room_later end def broadcast_remove broadcast_remove_to room, :messages end - private - # Fanned out to the room's members rather than published on one global stream, so - # that the timing of activity in a room only reaches people who are in it. - def broadcast_unread_room - payload = ActiveSupport::JSON.encode(roomId: room.id) + # Fanned out to the room's members rather than published on one global stream, so + # that the timing of activity in a room only reaches people who are in it. + def broadcast_unread_room + payload = ActiveSupport::JSON.encode(roomId: room.id) - room.memberships.pluck(:user_id).each do |user_id| - ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(user_id), payload, coder: nil - end + room.memberships.pluck(:user_id).each do |user_id| + ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(user_id), payload, coder: nil + end + end + + private + # The fanout is one publish per member, which in a big room takes far longer than the + # rest of posting a message, so it runs in a job rather than while the poster waits. + def broadcast_unread_room_later + Message::BroadcastUnreadRoomJob.perform_later(self) end end diff --git a/test/channels/unread_rooms_channel_test.rb b/test/channels/unread_rooms_channel_test.rb index 4444eb1..b3f36ed 100644 --- a/test/channels/unread_rooms_channel_test.rb +++ b/test/channels/unread_rooms_channel_test.rb @@ -16,7 +16,9 @@ class UnreadRoomsChannelTest < ActionCable::Channel::TestCase assert_not direct.users.include?(users(:jz)), "jz must be an outsider for this test to mean anything" broadcasts = capture_unread_broadcasts_for(users(:jz)) do - direct.messages.create!(body: "Private", creator: users(:kevin), client_message_id: "outsider").broadcast_create + perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob do + direct.messages.create!(body: "Private", creator: users(:kevin), client_message_id: "outsider").broadcast_create + end end assert_empty broadcasts @@ -26,7 +28,9 @@ class UnreadRoomsChannelTest < ActionCable::Channel::TestCase direct = rooms(:bender_and_kevin) broadcasts = capture_unread_broadcasts_for(users(:kevin)) do - direct.messages.create!(body: "Private", creator: users(:bender), client_message_id: "member").broadcast_create + perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob do + direct.messages.create!(body: "Private", creator: users(:bender), client_message_id: "member").broadcast_create + end end assert_equal [ direct.id ], broadcasts.collect { |broadcast| broadcast["roomId"] } diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb index 8088694..63266cb 100644 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@ -60,18 +60,32 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest test "creating a message broadcasts unread room to each member" do @room.users.each do |member| assert_broadcasts UnreadRoomsChannel.stream_name_for(member.id), 1 do - post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one #{member.id}", client_message_id: member.id } } + perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob do + post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one #{member.id}", client_message_id: member.id } } + end end end end + test "creating a message leaves the unread fanout to a job" do + member = @room.users.excluding(users(:david)).first + + assert_no_broadcasts UnreadRoomsChannel.stream_name_for(member.id) do + post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } } + end + + assert_enqueued_with job: Message::BroadcastUnreadRoomJob, args: [ Message.last ] + end + test "creating a message doesn't broadcast unread room to non-members" do outsiders = User.where.not(id: @room.users.map(&:id)) assert outsiders.any?, "need someone outside the room for this test to mean anything" outsiders.each do |outsider| assert_no_broadcasts UnreadRoomsChannel.stream_name_for(outsider.id) do - post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } } + perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob do + post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } } + end end end end diff --git a/test/system/unread_rooms_test.rb b/test/system/unread_rooms_test.rb index 235e912..ea68ba0 100644 --- a/test/system/unread_rooms_test.rb +++ b/test/system/unread_rooms_test.rb @@ -15,8 +15,11 @@ class UnreadRoomsTest < ApplicationSystemTestCase using_session("Kevin") do sign_in "kevin@37signals.com" join_room designers_room - send_message("Hello!!") - send_message("Talking to myself?") + + perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob do + send_message("Hello!!") + send_message("Talking to myself?") + end end assert_room_unread designers_room