From 80ab16a2d3e097686a2b02016a3de691c359fe93 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Wed, 18 Oct 2017 02:09:45 +0200 Subject: [PATCH] Improved performance of ticket overview backend job. --- lib/sessions/backend/activity_stream.rb | 2 +- lib/sessions/backend/base.rb | 37 +++--- lib/sessions/backend/collections/base.rb | 2 +- lib/sessions/backend/ticket_overview_list.rb | 6 +- test/unit/assets_test.rb | 2 +- test/unit/session_basic_ticket_test.rb | 119 +++++++++++++++++-- test/unit/session_collections_test.rb | 8 ++ 7 files changed, 142 insertions(+), 34 deletions(-) diff --git a/lib/sessions/backend/activity_stream.rb b/lib/sessions/backend/activity_stream.rb index 10f6874bc..acc463dd0 100644 --- a/lib/sessions/backend/activity_stream.rb +++ b/lib/sessions/backend/activity_stream.rb @@ -44,7 +44,7 @@ class Sessions::Backend::ActivityStream data = load - return if !data || data.empty? + return if data.blank? if !@client return { diff --git a/lib/sessions/backend/base.rb b/lib/sessions/backend/base.rb index afbb67aa8..fa0ef90bd 100644 --- a/lib/sessions/backend/base.rb +++ b/lib/sessions/backend/base.rb @@ -9,25 +9,28 @@ class Sessions::Backend::Base @last_change = nil end - def asset_needed?(record) + def asset_push(record, assets) class_name = record.class.to_s - if !@asset_lookup || !@asset_lookup[class_name] || !@asset_lookup[class_name][record.id] - @asset_lookup[class_name] ||= {} - @asset_lookup[class_name][record.id] = { - updated_at: record.updated_at, - pushed_at: Time.zone.now, - } - return true - end + @asset_lookup[class_name] ||= {} + @asset_lookup[class_name][record.id] = { + updated_at: record.updated_at, + pushed_at: Time.zone.now, + } + record.assets(assets) + end - if (!@asset_lookup[class_name][record.id][:updated_at] || @asset_lookup[class_name][record.id][:updated_at] < record.updated_at) || - (!@asset_lookup[class_name][record.id][:pushed_at] || @asset_lookup[class_name][record.id][:pushed_at] > Time.zone.now - 45.seconds) - @asset_lookup[class_name][record.id] = { - updated_at: record.updated_at, - pushed_at: Time.zone.now, - } - return true - end + def asset_needed?(record) + return false if !asset_needed_by_updated_at?(record.class.to_s, record.id, record.updated_at) + true + end + + def asset_needed_by_updated_at?(class_name, record_id, updated_at) + return true if @asset_lookup.blank? + return true if @asset_lookup[class_name].blank? + return true if @asset_lookup[class_name][record_id].blank? + return true if @asset_lookup[class_name][record_id][:updated_at] < updated_at + return true if @asset_lookup[class_name][record_id][:pushed_at].blank? + return true if @asset_lookup[class_name][record_id][:pushed_at] < Time.zone.now - 1.hour false end diff --git a/lib/sessions/backend/collections/base.rb b/lib/sessions/backend/collections/base.rb index 1b3e526a2..c05da5383 100644 --- a/lib/sessions/backend/collections/base.rb +++ b/lib/sessions/backend/collections/base.rb @@ -54,7 +54,7 @@ class Sessions::Backend::Collections::Base < Sessions::Backend::Base assets = {} items.each do |item| next if !asset_needed?(item) - assets = item.assets(assets) + assets = asset_push(item, assets) end if !@client return { diff --git a/lib/sessions/backend/ticket_overview_list.rb b/lib/sessions/backend/ticket_overview_list.rb index 806b6fe4f..d786619f2 100644 --- a/lib/sessions/backend/ticket_overview_list.rb +++ b/lib/sessions/backend/ticket_overview_list.rb @@ -98,12 +98,12 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base assets = {} overview = Overview.lookup(id: data[:overview][:id]) if asset_needed?(overview) - assets = overview.assets(assets) + assets = asset_push(overview, assets) end data[:tickets].each do |ticket_meta| + next if !asset_needed_by_updated_at?('Ticket', ticket_meta[:id], ticket_meta[:updated_at]) ticket = Ticket.lookup(id: ticket_meta[:id]) - next if !asset_needed?(ticket) - assets = ticket.assets(assets) + assets = asset_push(ticket, assets) end data[:assets] = assets diff --git a/test/unit/assets_test.rb b/test/unit/assets_test.rb index 84d098dae..9e7914d7b 100644 --- a/test/unit/assets_test.rb +++ b/test/unit/assets_test.rb @@ -419,7 +419,7 @@ class AssetsTest < ActiveSupport::TestCase assert_not(assets[:User][user5.id], 'check assets') assert(assets[:TicketState][ticket_state1.id], 'check assets') assert_not(assets[:TicketState][ticket_state2.id], 'check assets') - + overview.destroy! end test 'sla' do diff --git a/test/unit/session_basic_ticket_test.rb b/test/unit/session_basic_ticket_test.rb index 7cc0d8ddc..8cd707a6d 100644 --- a/test/unit/session_basic_ticket_test.rb +++ b/test/unit/session_basic_ticket_test.rb @@ -3,12 +3,12 @@ require 'test_helper' class SessionBasicTicketTest < ActiveSupport::TestCase - test 'b ticket_overview_List' do + setup do UserInfo.current_user_id = 1 roles = Role.where(name: ['Agent']) groups = Group.all - agent1 = User.create_or_update( + @agent1 = User.create_or_update( login: 'session-basic-ticket-agent-1', firstname: 'Session', lastname: 'session basic ' + rand(99_999).to_s, @@ -18,15 +18,90 @@ class SessionBasicTicketTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - assert(agent1.save!, 'create/update agent1') - Ticket.create(title: 'default overview test', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) + @agent2 = User.create_or_update( + login: 'session-basic-ticket-agent-2', + firstname: 'Session', + lastname: 'session basic ' + rand(99_999).to_s, + email: 'session-basic-ticket-agent-2@example.com', + password: 'agentpw', + active: true, + roles: roles, + groups: groups, + ) + Overview.destroy_all + load "#{Rails.root}/db/seeds/overviews.rb" + end - user = User.lookup(id: agent1.id) - client1 = Sessions::Backend::TicketOverviewList.new(user, {}, false, '123-1', 2) + test 'asset needed' do + + client1 = Sessions::Backend::TicketOverviewList.new(@agent1, {}, false, '123-1', 2) + client2 = Sessions::Backend::TicketOverviewList.new(@agent2, {}, false, '123-2', 2) + + ticket = Ticket.create!(title: 'default overview test', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) + + assert(client1.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client1.asset_push(ticket, {}) + assert_not(client1.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + + assert(client2.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client2.asset_push(ticket, {}) + assert_not(client2.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + + travel 30.minutes + + assert_not(client1.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + assert_not(client2.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + + travel 60.minutes + + assert(client1.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client1.asset_push(ticket, {}) + assert(client2.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client2.asset_push(ticket, {}) + + assert_not(client1.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client1.asset_push(ticket, {}) + assert_not(client2.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client2.asset_push(ticket, {}) + + ticket.touch + + assert(client1.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client1.asset_push(ticket, {}) + assert(client2.asset_needed_by_updated_at?(ticket.class.to_s, ticket.id, ticket.updated_at)) + client2.asset_push(ticket, {}) + + assert_not(client1.asset_needed?(ticket)) + assert_not(client2.asset_needed?(ticket)) + + travel 65.minutes + + assert(client1.asset_needed?(ticket)) + client1.asset_push(ticket, {}) + assert(client2.asset_needed?(ticket)) + client2.asset_push(ticket, {}) + + assert_not(client1.asset_needed?(ticket)) + assert_not(client2.asset_needed?(ticket)) + + travel_back + end + + test 'ticket_overview_List' do + + ticket1 = Ticket.create!(title: 'default overview test 1', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) + ticket2 = Ticket.create!(title: 'default overview test 2', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) + + client1 = Sessions::Backend::TicketOverviewList.new(@agent1, {}, false, '123-1', 2) result1 = client1.push assert(result1, 'check ticket_overview_List') + assert(result1[1][:data][:assets]) + assert(result1[1][:data][:assets][:Overview]) + assert(result1[1][:data][:assets][:User]) + assert_equal(result1[1][:data][:assets][:Ticket][ticket1.id]['title'], ticket1.title) + assert_equal(result1[1][:data][:assets][:Ticket][ticket2.id]['title'], ticket2.title) # next check should be empty / no changes result1 = client1.push @@ -38,33 +113,55 @@ class SessionBasicTicketTest < ActiveSupport::TestCase assert(!result1, 'check ticket_overview_index - recall 2') # create ticket - ticket = Ticket.create(title: '12323', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) + ticket3 = Ticket.create!(title: '12323', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) travel 3.seconds result1 = client1.push assert(result1, 'check ticket_overview_index - recall 3') + assert(result1[1][:data][:assets]) + assert_not(result1[1][:data][:assets][:Ticket][ticket1.id]) + assert_not(result1[1][:data][:assets][:Ticket][ticket2.id]) + assert_equal(result1[1][:data][:assets][:Ticket][ticket3.id]['title'], ticket3.title) + travel 3.seconds # chnage overview overviews = Ticket::Overviews.all( - current_user: user, + current_user: @agent1, ) overviews.first.touch result1 = client1.push assert(result1, 'check ticket_overview_index - recall 4') + assert(result1[1][:data][:assets]) + assert_not(result1[1][:data][:assets][:Ticket]) result1 = client1.push assert(!result1, 'check ticket_overview_index - recall 5') - Sessions::Backend::TicketOverviewList.reset(user.id) + Sessions::Backend::TicketOverviewList.reset(@agent1.id) result1 = client1.push assert(!result1, 'check ticket_overview_index - recall 6') - ticket = Ticket.create(title: '12323 - 2', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) - Sessions::Backend::TicketOverviewList.reset(user.id) + ticket4 = Ticket.create!(title: '12323 - 2', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) + Sessions::Backend::TicketOverviewList.reset(@agent1.id) result1 = client1.push assert(result1, 'check ticket_overview_index - recall 7') + assert(result1[1][:data][:assets]) + assert_not(result1[1][:data][:assets][:Ticket][ticket1.id]) + assert_not(result1[1][:data][:assets][:Ticket][ticket2.id]) + assert_not(result1[1][:data][:assets][:Ticket][ticket3.id]) + assert_equal(result1[1][:data][:assets][:Ticket][ticket4.id]['title'], ticket4.title) + + travel 65.minutes + ticket1.touch + result1 = client1.push + assert(result1, 'check ticket_overview_index - recall 8') + assert(result1[1][:data][:assets]) + assert_equal(result1[1][:data][:assets][:Ticket][ticket1.id]['title'], ticket1.title) + assert_equal(result1[1][:data][:assets][:Ticket][ticket2.id]['title'], ticket2.title) + assert_equal(result1[1][:data][:assets][:Ticket][ticket3.id]['title'], ticket3.title) + assert_equal(result1[1][:data][:assets][:Ticket][ticket4.id]['title'], ticket4.title) travel 10.seconds Sessions.destroy_idle_sessions(3) diff --git a/test/unit/session_collections_test.rb b/test/unit/session_collections_test.rb index 621531b4c..c4fc2caa2 100644 --- a/test/unit/session_collections_test.rb +++ b/test/unit/session_collections_test.rb @@ -194,6 +194,14 @@ class SessionCollectionsTest < ActiveSupport::TestCase assert(data[:assets][:Group][groups.first.id]) travel 10.seconds + client1 = Sessions::Backend::Collections::Group.new(agent1, assets, false, '123-1', 4) + data = client1.push + assert_equal(data[:collection][:Group][0]['id'], groups[0].id) + assert(data[:assets]) + assert_not(data[:assets][:Group]) + + travel 65.minutes + client1 = Sessions::Backend::Collections::Group.new(agent1, assets, false, '123-1', 4) data = client1.push assert_equal(data[:collection][:Group][0]['id'], groups[0].id)