diff --git a/app/models/rooms/direct.rb b/app/models/rooms/direct.rb index ba7856b..18430d5 100644 --- a/app/models/rooms/direct.rb +++ b/app/models/rooms/direct.rb @@ -7,12 +7,15 @@ class Rooms::Direct < Room end private - # FIXME: Find a more performant algorithm that won't be a problem on accounts with 10K+ direct rooms, - # which could be to store the membership id list as a hash on the room, and use that for lookup. + # Among the first user's rooms, the one whose members are exactly these users: as many + # memberships as users, and all of them theirs. def find_for(users) - all.joins(:users).detect do |room| - Set.new(room.user_ids) == Set.new(users.pluck(:id)) - end + user_ids = users.pluck(:id).uniq + + where(id: Membership.where(user_id: user_ids.first).select(:room_id)) + .joins(:memberships).group(:id) + .having("COUNT(*) = :size AND COUNT(CASE WHEN memberships.user_id IN (:user_ids) THEN 1 END) = :size", size: user_ids.size, user_ids: user_ids) + .first end end diff --git a/test/models/rooms/direct_test.rb b/test/models/rooms/direct_test.rb index ab74efa..b3f2722 100644 --- a/test/models/rooms/direct_test.rb +++ b/test/models/rooms/direct_test.rb @@ -18,4 +18,33 @@ class Rooms::DirectTest < ActiveSupport::TestCase room = Rooms::Direct.find_or_create_for([ users(:david), users(:kevin) ]) assert room.memberships.all? { |m| m.involved_in_everything? } end + + test "find the room with exactly the same users" do + Current.user = users(:david) + group = Rooms::Direct.find_or_create_for([ users(:david), users(:kevin), users(:jason) ]) + + assert_equal rooms(:david_and_kevin), Rooms::Direct.find_or_create_for([ users(:kevin), users(:david) ]) + assert_equal group, Rooms::Direct.find_or_create_for(User.where(id: [ users(:jason), users(:kevin), users(:david) ])) + assert_equal [ users(:david), users(:jz) ].to_set, Rooms::Direct.find_or_create_for([ users(:david), users(:jz) ]).users.to_set + assert_equal [ users(:jason), users(:kevin) ].to_set, Rooms::Direct.find_or_create_for([ users(:jason), users(:kevin) ]).users.to_set + end + + test "finding a room takes the same queries however many direct rooms the account has" do + Current.user = users(:david) + Rooms::Direct.create_for({}, users: [ users(:david), users(:jz) ]) + before = queries_during { Rooms::Direct.find_or_create_for(User.where(id: [ users(:david), users(:jz) ])) } + + 5.times { |i| Rooms::Direct.create_for({}, users: [ users(:jason), User.create!(name: "Guest #{i}", email_address: "guest#{i}@example.com", password: "secret123456") ]) } + Rooms::Direct.create_for({}, users: [ users(:kevin), users(:jz) ]) + + assert_equal before, queries_during { Rooms::Direct.find_or_create_for(User.where(id: [ users(:kevin), users(:jz) ])) } + end + + private + def queries_during(&block) + count = 0 + counter = ->(*, payload) { count += 1 unless payload[:name] == "SCHEMA" || payload[:cached] } + ActiveSupport::Notifications.subscribed(counter, "sql.active_record", &block) + count + end end