Browse Source

Fixes #3586 - SessionTimeoutJob.perform_now fails if user no longer exists.

Rolf Schmidt 3 years ago
parent
commit
313bd65212
2 changed files with 18 additions and 7 deletions
  1. 12 7
      app/jobs/session_timeout_job.rb
  2. 6 0
      spec/jobs/session_timeout_job_spec.rb

+ 12 - 7
app/jobs/session_timeout_job.rb

@@ -10,22 +10,27 @@ class SessionTimeoutJob < ApplicationJob
   def perform_session(session)
     return if !session.data['user_id']
 
-    user = User.find(session.data['user_id'])
-    return if !user
-
-    timeout = get_timeout(user)
-    return if session.data['ping'] > Time.zone.now - timeout.seconds
+    # user is optional because it can be deleted already
+    user = User.find_by(id: session.data['user_id'])
+    if user
+      timeout = get_timeout(user)
+      return if session.data['ping'] > timeout.seconds.ago
+    end
 
     self.class.destroy_session(user, session)
   end
 
   def self.destroy_session(user, session)
-    PushMessages.send_to(user.id, { event: 'session_timeout' })
+
+    # user is optional because it can be deleted already
+    if user
+      PushMessages.send_to(user.id, { event: 'session_timeout' })
+    end
     session.destroy
   end
 
   def sessions
-    ActiveRecord::SessionStore::Session.where('updated_at < ?', Time.zone.now - config.values.map(&:to_i).min.seconds)
+    ActiveRecord::SessionStore::Session.where('updated_at < ?', config.values.map(&:to_i).min.seconds.ago)
   end
 
   def config

+ 6 - 0
spec/jobs/session_timeout_job_spec.rb

@@ -19,6 +19,12 @@ RSpec.describe SessionTimeoutJob, type: :job do
       expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(-1)
     end
 
+    it 'does also kill the session of deleted users' do
+      user.destroy
+      travel_to 1.hour.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(-1)
+    end
+
     it 'does not kill the session' do
       travel_to 1.minute.from_now
       expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(0)