Maintenance: Improve session takeover handling.

.rubocop/todo.yml

@@ -325,7 +325,6 @@ Metrics/AbcSize:
     - 'lib/sessions/backend/ticket_overview_list.rb'
     - 'lib/sessions/client.rb'
     - 'lib/sessions/event.rb'
-    - 'lib/sessions/event/broadcast.rb'
     - 'lib/sessions/event/chat_session_close.rb'
     - 'lib/sessions/event/chat_session_init.rb'
     - 'lib/sessions/event/chat_session_start.rb'
@@ -634,7 +633,6 @@ Metrics/CyclomaticComplexity:
     - 'lib/sessions/backend/activity_stream.rb'
     - 'lib/sessions/backend/ticket_overview_list.rb'
     - 'lib/sessions/client.rb'
-    - 'lib/sessions/event/broadcast.rb'
     - 'lib/sessions/event/chat_session_close.rb'
     - 'lib/sessions/event/chat_session_update.rb'
     - 'lib/sessions/event/chat_status_customer.rb'
@@ -857,7 +855,6 @@ Metrics/PerceivedComplexity:
     - 'lib/sessions/backend/activity_stream.rb'
     - 'lib/sessions/backend/ticket_overview_list.rb'
     - 'lib/sessions/client.rb'
-    - 'lib/sessions/event/broadcast.rb'
     - 'lib/sessions/event/chat_session_close.rb'
     - 'lib/sessions/event/chat_session_update.rb'
     - 'lib/sessions/event/chat_status_customer.rb'

app/assets/javascripts/app/controllers/_plugin/session_taken_over.coffee

@@ -8,24 +8,17 @@ class SessionTakeOver extends App.Controller
       =>
         @spoolSent = true
 
-        # broadcast to other browser instance
         App.WebSocket.send(
-          event: 'broadcast'
-          spool:  true
-          recipient:
-            user_id: [ App.Session.get( 'id' ) ]
+          event: 'session_takeover',
           data:
-            event: 'session:takeover'
-            data:
-              taskbar_id: App.TaskManager.TaskbarId()
+            taskbar_id: App.TaskManager.TaskbarId()
         )
     )
 
     # session take over message
     @controllerBind(
-      'session:takeover'
+      'session_takeover'
       (data) =>
-
         # only if spool messages are already sent
         return if !@spoolSent
 

lib/sessions.rb

@@ -230,7 +230,7 @@ send message to recipient client
 e. g.
 
   Sessions.send_to(user_id, {
-    event: 'session:takeover',
+    event: 'session_takeover',
     data: {
       taskbar_id: 12312
     },

lib/sessions/event/broadcast.rb

@@ -1,53 +0,0 @@
-# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
-
-class Sessions::Event::Broadcast < Sessions::Event::Base
-
-=begin
-
-Event module to broadcast messages to all client connections.
-
-To execute this manually, just paste the following into the browser console
-
-  App.WebSocket.send({event:'broadcast', recipient: { user_id: [1,2,3]}, data: {some: 'key'}})
-
-=end
-
-  def run
-
-    # list all current clients
-    client_list = Sessions.list
-    client_list.each do |local_client_id, local_client|
-      if local_client_id == @client_id
-        log 'info', 'do not send broadcast to itself'
-        next
-      end
-
-      # broadcast to recipient list
-      if @payload['recipient']
-        if @payload['recipient'].class != Hash && @payload['recipient'].class != ActiveSupport::HashWithIndifferentAccess && @payload['recipient'].class != ActionController::Parameters
-          log 'error', "recipient attribute isn't a hash (#{@payload['recipient'].class}) '#{@payload['recipient'].inspect}'"
-        elsif !@payload['recipient'].key?('user_id')
-          log 'error', "need recipient.user_id attribute '#{@payload['recipient'].inspect}'"
-        elsif @payload['recipient']['user_id'].class != Array
-          log 'error', "recipient.user_id attribute isn't an array '#{@payload['recipient']['user_id'].inspect}'"
-        else
-          @payload['recipient']['user_id'].each do |user_id|
-
-            next if local_client[:user]['id'].to_i != user_id.to_i
-
-            log 'info', "send broadcast from (#{@client_id}) to (user_id=#{user_id})", local_client_id
-            websocket_send(local_client_id, @payload['data'])
-          end
-        end
-
-        # broadcast every client
-      else
-        log 'info', "send broadcast from (#{@client_id})", local_client_id
-        websocket_send(local_client_id, @payload['data'])
-      end
-    end
-
-    false
-  end
-
-end

lib/sessions/event/session_takeover.rb

@@ -0,0 +1,15 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+class Sessions::Event::SessionTakeover < Sessions::Event::Base
+  database_connection_required
+
+  def run
+    return if !valid_session?
+
+    Sessions.send_to(@session['id'], {
+                       event: 'session_takeover',
+                       data:  @payload['data'],
+                     })
+  end
+
+end

lib/sessions/event/ticket_overview_select.rb

@@ -4,11 +4,11 @@ class Sessions::Event::TicketOverviewSelect < Sessions::Event::Base
 
 =begin
 
-Event module to serve spool messages and send them to new client connection.
+Observing every ticket overview of each agent session does not scale well on larger systems (e.g. 60 ticket overviews per agent).
+With this change, only the five most recently used ones are checked on every iteration.
+A full check is still performed (every 60 seconds). This reduces the overall load.
 
-To execute this manually, just paste the following into the browser console
-
-  App.WebSocket.send({event:'spool'})
+  App.WebSocket.send({event:'ticket_overview_select'}, data: { view: ''})
 
 =end
 

spec/requests/long_polling_spec.rb

@@ -105,15 +105,8 @@ RSpec.describe 'LongPolling', type: :request do
       spool_list = Sessions.spool_list(Time.now.utc.to_i, agent.id)
       expect(spool_list).to eq([])
 
-      get '/api/v1/message_send', params: { client_id: client_id, data: { event: 'broadcast', spool: true, recipient: { user_id: [agent.id] }, data: { taskbar_id: 9_391_633 } } }, as: :json
+      get '/api/v1/message_send', params: { client_id: client_id, data: { event: 'session_takeover', spool: true, data: { taskbar_id: 9_391_633 } } }, as: :json
       expect(response).to have_http_status(:ok)
-      expect(json_response).to be_a(Hash)
-      expect(json_response).to eq({})
-
-      get '/api/v1/message_receive', params: { client_id: client_id, data: {} }, as: :json
-      expect(response).to have_http_status(:ok)
-      expect(json_response).to be_a(Hash)
-      expect(json_response).to eq({ 'event' => 'pong' })
 
       travel 2.seconds
 
@@ -121,8 +114,7 @@ RSpec.describe 'LongPolling', type: :request do
       expect(spool_list).to eq([])
 
       spool_list = Sessions.spool_list(nil, agent.id)
-      expect(spool_list).to eq([{ message: { 'taskbar_id' => 9_391_633 }, type: 'direct' }])
-
+      expect(spool_list).to eq([{ message: { 'data' => { 'taskbar_id'=>9_391_633 }, 'event' => 'session_takeover', 'spool' => true }, type: 'broadcast' }])
     end
 
     it 'automatically cleans-up old spool entries' do

spec/system/basic/session_takeover_spec.rb

@@ -0,0 +1,23 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Session takeover check', type: :system do
+  context 'when use logout action' do
+    let(:agent)    { create(:agent) }
+
+    it 'check that all tabs have been logged out', authenticated_as: :agent do
+      visit '/'
+
+      # open new tab
+      open_window_and_switch
+
+      visit '/'
+
+      # Go back and check for session takeover message
+      switch_to_window_index(1)
+
+      expect(page).to have_text('A new session was created with your account. This session will be stopped to prevent a conflict.')
+    end
+  end
+end