From 6018d864dbdd24a372cef76a1f03b08670aeb030 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Wed, 2 Oct 2019 15:57:08 +0200 Subject: [PATCH] Performance: `Sessions::Backend::TicketOverivewList` check whether data has to be sent to the client was wrong. The correct check now compares the list of tickets instead of comparing the whole hash structure including assets. --- lib/sessions/backend/ticket_overview_list.rb | 4 +- .../backend/ticket_overview_list_spec.rb | 63 +++---------------- test/browser_test_helper.rb | 24 ++++++- 3 files changed, 33 insertions(+), 58 deletions(-) diff --git a/lib/sessions/backend/ticket_overview_list.rb b/lib/sessions/backend/ticket_overview_list.rb index 381057e9c..7a5133e2e 100644 --- a/lib/sessions/backend/ticket_overview_list.rb +++ b/lib/sessions/backend/ticket_overview_list.rb @@ -89,9 +89,9 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base index_and_lists.each do |data| # do not deliver unchanged lists - next if @last_overview[data[:overview][:id]] == data + next if @last_overview[data[:overview][:id]] == [data[:tickets], data[:overview]] - @last_overview[data[:overview][:id]] = data + @last_overview[data[:overview][:id]] = [data[:tickets], data[:overview]] assets = {} overview = Overview.lookup(id: data[:overview][:id]) diff --git a/spec/lib/sessions/backend/ticket_overview_list_spec.rb b/spec/lib/sessions/backend/ticket_overview_list_spec.rb index ef9117e54..53b53ef5e 100644 --- a/spec/lib/sessions/backend/ticket_overview_list_spec.rb +++ b/spec/lib/sessions/backend/ticket_overview_list_spec.rb @@ -62,8 +62,8 @@ RSpec.describe Sessions::Backend::TicketOverviewList do end context 'when called twice, after changes have occurred to the Ticket table' do + let!(:ticket) { create(:ticket, group: group) } let!(:first_call) { collection.push } - let!(:ticket) { create(:ticket, group: group, owner: admin) } context 'before the TTL has passed' do it 'returns nil' do @@ -73,16 +73,8 @@ RSpec.describe Sessions::Backend::TicketOverviewList do context 'after .reset with the user’s id' do before { described_class.reset(admin.id) } - it 'returns an updated set of results' do - expect(collection.push) - .to be_an(Array) - .and have_attributes(length: Ticket::Overviews.all(current_user: admin).count) - .and all(match({ event: 'ticket_overview_list', data: hash_including(:assets) })) - end - - it 'includes FE assets for the new ticket' do - expect(collection.push.first[:data][:assets]) # first overview is "My assigned tickets", - .to eq(ticket.assets({})) # and newly created ticket was assigned to admin + it 'returns nil because no ticket and no overview has changed' do + expect(collection.push).to be nil end end end @@ -90,32 +82,16 @@ RSpec.describe Sessions::Backend::TicketOverviewList do context 'after the TTL has passed' do before { travel(ttl + 1) } - it 'returns an updated set of results' do - expect(collection.push) - .to be_an(Array) - .and have_attributes(length: Ticket::Overviews.all(current_user: admin).count) - .and all(match({ event: 'ticket_overview_list', data: hash_including(:assets) })) - end - - it 'includes FE assets for the new ticket' do - expect(collection.push.first[:data][:assets]) # first overview is "My assigned tickets", - .to eq(ticket.assets({})) # and newly created ticket was assigned to admin + it 'returns an empty result' do + expect(collection.push).to eq nil end end context 'after two hours have passed' do before { travel(2.hours + 1.second) } - it 'returns an updated set of results' do - expect(collection.push) - .to be_an(Array) - .and have_attributes(length: Ticket::Overviews.all(current_user: admin).count) - .and all(match({ event: 'ticket_overview_list', data: hash_including(:assets) })) - end - - it 'includes FE assets for old AND new tickets/overviews' do - expect(collection.push.first[:data][:assets]) # first overview is "My assigned tickets", - .to eq(Overview.first.assets(ticket.assets({}))) # and newly created ticket was assigned to admin + it 'returns an empty result' do + expect(collection.push).to eq nil end end end @@ -136,40 +112,21 @@ RSpec.describe Sessions::Backend::TicketOverviewList do it 'returns an updated set of results' do expect(collection.push) .to be_an(Array) - .and have_attributes(length: Ticket::Overviews.all(current_user: admin).count) + .and have_attributes(length: 1) .and all(match({ event: 'ticket_overview_list', data: hash_including(:assets) })) end - - it 'includes FE assets for the touched overview' do - expect(collection.push.first[:data][:assets]) - .to eq(Overview.first.assets({})) - end - end - end - - context 'after the TTL has passed' do - before { travel(ttl + 1) } - - it 'includes FE assets for the touched overview' do - expect(collection.push.first[:data][:assets]) - .to eq(Overview.first.assets({})) end end context 'after two hours have passed' do before { travel(2.hours + 1.second) } - it 'returns an updated set of results' do + it 'returns an empty result' do expect(collection.push) .to be_an(Array) - .and have_attributes(length: Ticket::Overviews.all(current_user: admin).count) + .and have_attributes(length: 1) .and all(match({ event: 'ticket_overview_list', data: hash_including(:assets) })) end - - it 'includes FE assets for old AND new tickets/overviews' do - expect(collection.push.first[:data][:assets]) - .to eq(Overview.first.assets({})) - end end end end diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index 3e05e13e2..b93912e66 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -2665,7 +2665,14 @@ wait untill text in selector disabppears end # switch to overview - instance.find_elements(css: ".content.active .sidebar a[href=\"#{link}\"]")[0].click + element = nil + 6.times do + element = instance.find_elements(css: ".content.active .sidebar a[href=\"#{link}\"]")[0] + break if element + + sleep 1 + end + element.click # hide larger overview selection list again sleep 0.5 @@ -2700,14 +2707,25 @@ wait untill text in selector disabppears overview_open(params) + element = nil if params[:title] - element = instance.find_element(css: '.content.active').find_element(partial_link_text: params[:title]) + 6.times do + element = instance.find_element(css: '.content.active').find_element(partial_link_text: params[:title]) + break if element + + sleep 1 + end if !element screenshot(browser: instance, comment: 'ticket_open_by_overview_no_ticket_failed') raise "unable to find ticket #{params[:title]} in overview #{params[:link]}!" end else - element = instance.find_elements(partial_link_text: params[:number])[0] + 6.times do + element = instance.find_elements(partial_link_text: params[:number])[0] + break if element + + sleep 1 + end if !element screenshot(browser: instance, comment: 'ticket_open_by_overview_no_ticket_failed') raise "unable to find ticket #{params[:number]} in overview #{params[:link]}!"