Browse Source

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.

Martin Edenhofer 5 years ago
parent
commit
6018d864db

+ 2 - 2
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])

+ 10 - 53
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

+ 21 - 3
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]}!"