Skip to content

Fix an issue with user and last_activity pairs being mixed

Hannes Moser requested to merge fix-awareness-session-user-activity into master

What does this MR do and why?

The previous implementation of users_with_last_activity introduced a bug by ignoring that select * from users where id in(1, 2); statements do not guarantee to maintain the order in which IDs where provided to the in expression. This can lead to situations where the last_activity timestamp of one user is paired with a different user object.

Example

select * from users where id in(1, 2); # Result order: 2,1
select * from users where id in(2, 1); # Result order: 2,1

To fix this, a stable order criteria is added to both, the result returned from the Redis store and the set of users queried with ActiveRecord. Both results are ordered by ID in ascending order.

def users_with_last_activity
    # where in (x, y, …) is a set and does not maintain any order, we need to
    # make sure to establish a stable order for both, the pairs returned from
    # redis and the ActiveRecord query. Using IDs in ascending order.
    user_ids, last_activities = user_ids_with_last_activity
      .sort_by(&:first)
      .transpose
    users = User.where(id: user_ids).order(id: :asc)
    users.zip(last_activities)
  end

How to set up and validate locally

Added a spec to test for the expected behavior.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Hannes Moser

Merge request reports

Loading