From e37328d2019bc0fac17a30354dfbfe577f91921c Mon Sep 17 00:00:00 2001
From: Thorsten Eckel <te@znuny.com>
Date: Mon, 8 Jan 2018 16:30:09 +0100
Subject: [PATCH] Refactoring: Migrated LDAP and Excachange statistics to new
 sum/total structure for clearer results and better performance.

---
 .../controllers/_integration/exchange.coffee  | 21 +++++-----------
 .../app/controllers/_integration/ldap.coffee  | 24 +++++--------------
 .../integration/exchange_last_import.jst.eco  |  8 +++----
 .../integration/exchange_summary.jst.eco      |  2 +-
 .../integration/ldap_last_import.jst.eco      |  8 +++----
 .../views/integration/ldap_summary.jst.eco    |  2 +-
 .../import/exchange/folder_contacts.rb        |  2 +-
 lib/sequencer/sequence/import/ldap/users.rb   |  2 +-
 .../folder_contact/statistics/diff.rb         |  8 ++-----
 .../folder_contacts/{sum.rb => total.rb}      | 22 +++++++++--------
 .../unit/import/ldap/user/statistics/diff.rb  |  6 +----
 .../import/ldap/users/{sum.rb => total.rb}    |  6 ++---
 12 files changed, 42 insertions(+), 69 deletions(-)
 rename lib/sequencer/unit/import/exchange/folder_contacts/{sum.rb => total.rb} (61%)
 rename lib/sequencer/unit/import/ldap/users/{sum.rb => total.rb} (91%)

diff --git a/app/assets/javascripts/app/controllers/_integration/exchange.coffee b/app/assets/javascripts/app/controllers/_integration/exchange.coffee
index c02c541ba..13ea97709 100644
--- a/app/assets/javascripts/app/controllers/_integration/exchange.coffee
+++ b/app/assets/javascripts/app/controllers/_integration/exchange.coffee
@@ -131,13 +131,12 @@ class Form extends App.Controller
     if _.isEmpty(job)
       @lastImport.html('')
       return
-    countDone = job.result.created + job.result.updated + job.result.unchanged + job.result.skipped + job.result.failed
     if !job.result.roles
       job.result.roles = {}
     for role_id, statistic of job.result.role_ids
       role = App.Role.find(role_id)
       job.result.roles[role.displayName()] = statistic
-    el = $(App.view('integration/exchange_last_import')(job: job, countDone: countDone))
+    el = $(App.view('integration/exchange_last_import')(job: job))
     @lastImport.html(el)
 
   activeDryRun: =>
@@ -540,34 +539,26 @@ class ConnectionWizard extends App.WizardModal
           @showAlert('js-error', (job.result.error || job.result.info))
           return
 
-        total = 0
         if job.result && _.keys(job.result).length > 0
           @$('.js-preprogress').addClass('hide')
           @$('.js-analyzing').removeClass('hide')
 
-          analized = 0
-          total    = job.result.sum
-          for action, sum of job.result
-            continue if action == 'folders'
-            continue if action == 'sum'
-            analized += sum
-
-          @$('.js-progress progress').attr('value', analized)
-          @$('.js-progress progress').attr('max', total)
+          @$('.js-progress progress').attr('value', job.result.sum)
+          @$('.js-progress progress').attr('max', job.result.total)
 
         if job.finished_at
           # reset initial state in case the back button is used
           @$('.js-preprogress').removeClass('hide')
           @$('.js-analyzing').addClass('hide')
 
-          @tryResult(job, total)
+          @tryResult(job)
         else
           @delay(@tryLoop, 4000)
     )
 
-  tryResult: (job, total) =>
+  tryResult: (job) =>
     @showSlide('js-try')
-    el = $(App.view('integration/exchange_summary')(job: job, countDone: total))
+    el = $(App.view('integration/exchange_summary')(job: job))
     @el.find('.js-summary').html(el)
 
 App.Config.set(
diff --git a/app/assets/javascripts/app/controllers/_integration/ldap.coffee b/app/assets/javascripts/app/controllers/_integration/ldap.coffee
index cd6312a68..3384980a9 100644
--- a/app/assets/javascripts/app/controllers/_integration/ldap.coffee
+++ b/app/assets/javascripts/app/controllers/_integration/ldap.coffee
@@ -132,13 +132,12 @@ class Form extends App.Controller
     if _.isEmpty(job)
       @lastImport.html('')
       return
-    countDone = job.result.created + job.result.updated + job.result.unchanged + job.result.skipped + job.result.failed
     if !job.result.roles
       job.result.roles = {}
     for role_id, statistic of job.result.role_ids
       role = App.Role.find(role_id)
       job.result.roles[role.displayName()] = statistic
-    el = $(App.view('integration/ldap_last_import')(job: job, countDone: countDone))
+    el = $(App.view('integration/ldap_last_import')(job: job))
     @lastImport.html(el)
 
   activeDryRun: =>
@@ -539,22 +538,12 @@ class ConnectionWizard extends App.WizardModal
           @showAlert('js-error', (job.result.error || job.result.info))
           return
 
-        if job.result && job.result.sum
+        if job.result && job.result.total
           @$('.js-preprogress').addClass('hide')
           @$('.js-analyzing').removeClass('hide')
-          total = 0
-          if job.result.created
-            total += job.result.created
-          if job.result.failed
-            total += job.result.failed
-          if job.result.skipped
-            total += job.result.skipped
-          if job.result.unchanged
-            total += job.result.unchanged
-          if job.result.updated
-            total += job.result.updated
-          @$('.js-progress progress').attr('value', total)
-          @$('.js-progress progress').attr('max', job.result.sum)
+
+          @$('.js-progress progress').attr('value', job.result.sum)
+          @$('.js-progress progress').attr('max', job.result.total)
         if job.finished_at
           # reset initial state in case the back button is used
           @$('.js-preprogress').removeClass('hide')
@@ -574,9 +563,8 @@ class ConnectionWizard extends App.WizardModal
     for role_id, statistic of job.result.role_ids
       role = App.Role.find(role_id)
       job.result.roles[role.displayName()] = statistic
-    countDone = job.result.created + job.result.updated + job.result.unchanged + job.result.skipped
     @showSlide('js-try')
-    el = $(App.view('integration/ldap_summary')(job: job, countDone: countDone))
+    el = $(App.view('integration/ldap_summary')(job: job))
     @el.find('.js-summary').html(el)
 
 App.Config.set(
diff --git a/app/assets/javascripts/app/views/integration/exchange_last_import.jst.eco b/app/assets/javascripts/app/views/integration/exchange_last_import.jst.eco
index 8323972ad..4ded44e85 100644
--- a/app/assets/javascripts/app/views/integration/exchange_last_import.jst.eco
+++ b/app/assets/javascripts/app/views/integration/exchange_last_import.jst.eco
@@ -20,18 +20,18 @@
       <% if @job.result && @job.result.error: %>
         <p><%- @Ttimestamp(@job.started_at) %></p>
         <div class="alert alert--danger" role="alert"><%- @T('An error occurred: %s', @job.result.error) %></div>
-      <% else if !@countDone: %>
+      <% else if @job.result && !@job.result.sum: %>
         <p><%- @Ttimestamp(@job.started_at) %> - <%- @T('Counting entries. This may take a while.') %></p>
       <% else: %>
         <p><%- @Ttimestamp(@job.started_at) %> - <%- @T('Running...') %></p>
         <div class="flex">
-          <progress max="<%= @job.result.sum %>" value="<%= @countDone %>"></progress>
+          <progress max="<%= @job.result.total %>" value="<%= @job.result.sum %>"></progress>
         </div>
       <% end %>
     <% end %>
-    <% if !_.isEmpty(@job.result) && @countDone: %>
+    <% if !_.isEmpty(@job.result) && @job.result.sum: %>
       <ul>
-        <li><%- @T('%s user to %s user', 'Exchange', 'Zammad') %> (<%= @countDone %>/<%= @job.result.sum %>):
+        <li><%- @T('%s user to %s user', 'Exchange', 'Zammad') %> (<%= @job.result.sum %>/<%= @job.result.total %>):
           <ul>
             <li><%- @T('Users') %>: <%= @job.result.created %> <%- @T('created') %>, <%= @job.result.updated %> <%- @T('updated') %>, <%= @job.result.unchanged %> <%- @T('untouched') %>, <%= @job.result.skipped %> <%- @T('skipped') %>, <%= @job.result.failed %> <%- @T('failed') %>
           </ul>
diff --git a/app/assets/javascripts/app/views/integration/exchange_summary.jst.eco b/app/assets/javascripts/app/views/integration/exchange_summary.jst.eco
index 0abf47bdf..4407c355b 100644
--- a/app/assets/javascripts/app/views/integration/exchange_summary.jst.eco
+++ b/app/assets/javascripts/app/views/integration/exchange_summary.jst.eco
@@ -1,5 +1,5 @@
 <ul>
-  <li><%- @T('%s user to %s user', 'Exchange', 'Zammad') %> (<%= @countDone %>):
+  <li><%- @T('%s user to %s user', 'Exchange', 'Zammad') %> (<%= @job.result.total %>):
     <ul>
       <li><%- @T('Users') %>: <%= @job.result.created %> <%- @T('created') %>, <%= @job.result.updated %> <%- @T('updated') %>, <%= @job.result.unchanged %> <%- @T('untouched') %>, <%= @job.result.skipped %> <%- @T('skipped') %>, <%= @job.result.failed %> <%- @T('failed') %>
     </ul>
diff --git a/app/assets/javascripts/app/views/integration/ldap_last_import.jst.eco b/app/assets/javascripts/app/views/integration/ldap_last_import.jst.eco
index 75c44a713..a6c5e817a 100644
--- a/app/assets/javascripts/app/views/integration/ldap_last_import.jst.eco
+++ b/app/assets/javascripts/app/views/integration/ldap_last_import.jst.eco
@@ -20,18 +20,18 @@
       <% if @job.result && @job.result.error: %>
         <p><%- @Ttimestamp(@job.started_at) %></p>
         <div class="alert alert--danger" role="alert"><%- @T('An error occurred: %s', @job.result.error) %></div>
-      <% else if !@countDone: %>
+      <% else if !@job.result.sum: %>
         <p><%- @Ttimestamp(@job.started_at) %> - <%- @T('Counting entries. This may take a while.') %></p>
       <% else: %>
         <p><%- @Ttimestamp(@job.started_at) %> - <%- @T('Running...') %></p>
         <div class="flex">
-          <progress max="<%= @job.result.sum %>" value="<%= @countDone %>"></progress>
+          <progress max="<%= @job.result.total %>" value="<%= @job.result.sum %>"></progress>
         </div>
       <% end %>
     <% end %>
-    <% if !_.isEmpty(@job.result) && @countDone: %>
+    <% if !_.isEmpty(@job.result) && @job.result.sum: %>
       <ul>
-        <li><%- @T('%s user to %s user', 'LDAP', 'Zammad') %> (<%= @countDone %>/<%= @job.result.sum %>):
+        <li><%- @T('%s user to %s user', 'LDAP', 'Zammad') %> (<%= @job.result.sum %>/<%= @job.result.total %>):
           <ul>
             <li><%- @T('Users') %>: <%= @job.result.created %> <%- @T('created') %>, <%= @job.result.updated %> <%- @T('updated') %>, <%= @job.result.unchanged %> <%- @T('untouched') %>, <%= @job.result.skipped %> <%- @T('skipped') %>, <%= @job.result.failed %> <%- @T('failed') %>, <%= @job.result.deactivated %> <%- @T('deactivated') %>
           </ul>
diff --git a/app/assets/javascripts/app/views/integration/ldap_summary.jst.eco b/app/assets/javascripts/app/views/integration/ldap_summary.jst.eco
index 2b79071c1..f4d5b6aec 100644
--- a/app/assets/javascripts/app/views/integration/ldap_summary.jst.eco
+++ b/app/assets/javascripts/app/views/integration/ldap_summary.jst.eco
@@ -1,5 +1,5 @@
 <ul>
-  <li><%- @T('%s user to %s user', 'LDAP', 'Zammad') %> (<%= @countDone %>):
+  <li><%- @T('%s user to %s user', 'LDAP', 'Zammad') %> (<%= @job.result.sum %>):
     <ul>
       <li><%- @T('Users') %>: <%= @job.result.created %> <%- @T('created') %>, <%= @job.result.updated %> <%- @T('updated') %>, <%= @job.result.unchanged %> <%- @T('untouched') %>, <%= @job.result.skipped %> <%- @T('skipped') %>, <%= @job.result.failed %> <%- @T('failed') %>, <%= @job.result.deactivated %> <%- @T('deactivated') %>
     </ul>
diff --git a/lib/sequencer/sequence/import/exchange/folder_contacts.rb b/lib/sequencer/sequence/import/exchange/folder_contacts.rb
index 9d07f75cd..f07c503b3 100644
--- a/lib/sequencer/sequence/import/exchange/folder_contacts.rb
+++ b/lib/sequencer/sequence/import/exchange/folder_contacts.rb
@@ -9,7 +9,7 @@ class Sequencer
               'Import::Exchange::FolderContacts::DryRunPayload',
               'Exchange::Connection',
               'Import::Exchange::FolderContacts::FolderIds',
-              'Import::Exchange::FolderContacts::Sum',
+              'Import::Exchange::FolderContacts::Total',
               'Import::Common::ImportJob::Statistics::Update',
               'Import::Common::ImportJob::Statistics::Store',
               'Import::Exchange::FolderContacts::SubSequence',
diff --git a/lib/sequencer/sequence/import/ldap/users.rb b/lib/sequencer/sequence/import/ldap/users.rb
index 80d6eff9d..6b7772aac 100644
--- a/lib/sequencer/sequence/import/ldap/users.rb
+++ b/lib/sequencer/sequence/import/ldap/users.rb
@@ -13,7 +13,7 @@ class Sequencer
               'Ldap::Config',
               'Ldap::Connection',
               'Import::Ldap::Users::UserRoles',
-              'Import::Ldap::Users::Sum',
+              'Import::Ldap::Users::Total',
               'Import::Common::ImportJob::Statistics::Update',
               'Import::Common::ImportJob::Statistics::Store',
               'Import::Ldap::Users::SubSequence',
diff --git a/lib/sequencer/unit/import/exchange/folder_contact/statistics/diff.rb b/lib/sequencer/unit/import/exchange/folder_contact/statistics/diff.rb
index f7bedf2c3..857db7d97 100644
--- a/lib/sequencer/unit/import/exchange/folder_contact/statistics/diff.rb
+++ b/lib/sequencer/unit/import/exchange/folder_contact/statistics/diff.rb
@@ -11,15 +11,11 @@ class Sequencer
 
               def process
                 state.provide(:statistics_diff) do
-                  # remove :sum since it's already set via
-                  # the exchange item attribute
-                  result = diff.except(:sum)
-
                   # build structure for a general diff
                   # and a folder specific sub structure
-                  result.merge(
+                  diff.merge(
                     folders: {
-                      ews_folder_name => result
+                      ews_folder_name => diff
                     }
                   )
                 end
diff --git a/lib/sequencer/unit/import/exchange/folder_contacts/sum.rb b/lib/sequencer/unit/import/exchange/folder_contacts/total.rb
similarity index 61%
rename from lib/sequencer/unit/import/exchange/folder_contacts/sum.rb
rename to lib/sequencer/unit/import/exchange/folder_contacts/total.rb
index 0d940f8f8..5f1b03a25 100644
--- a/lib/sequencer/unit/import/exchange/folder_contacts/sum.rb
+++ b/lib/sequencer/unit/import/exchange/folder_contacts/total.rb
@@ -3,8 +3,9 @@ class Sequencer
     module Import
       module Exchange
         module FolderContacts
-          class Sum < Sequencer::Unit::Base
+          class Total < Sequencer::Unit::Base
             include ::Sequencer::Unit::Exchange::Folders::Mixin::Folder
+            include ::Sequencer::Unit::Import::Common::Model::Statistics::Mixin::Common
 
             uses :ews_folder_ids
             provides :statistics_diff
@@ -16,21 +17,22 @@ class Sequencer
             private
 
             def diff
-              result = {
-                sum: 0,
-              }
-              folder_sum_map.each do |display_path, sum|
+              result = empty_diff.merge(
+                folders: {},
+              )
 
-                result[display_path] = {
-                  sum: sum
-                }
+              folder_total_map.each do |display_path, total|
 
-                result[:sum] += sum
+                result[:folders][display_path] = empty_diff.merge(
+                  total: total
+                )
+
+                result[:total] += total
               end
               result
             end
 
-            def folder_sum_map
+            def folder_total_map
               ews_folder_ids.collect do |folder_id|
                 folder       = ews_folder.find(folder_id)
                 display_path = ews_folder.display_path(folder)
diff --git a/lib/sequencer/unit/import/ldap/user/statistics/diff.rb b/lib/sequencer/unit/import/ldap/user/statistics/diff.rb
index 82033d55d..9f07ab09c 100644
--- a/lib/sequencer/unit/import/ldap/user/statistics/diff.rb
+++ b/lib/sequencer/unit/import/ldap/user/statistics/diff.rb
@@ -13,11 +13,7 @@ class Sequencer
 
               def process
                 state.provide(:statistics_diff) do
-                  # remove :sum since it's already set via
-                  # the outer count Unit
-                  statistics = diff.except(:sum)
-
-                  add_role_ids(statistics)
+                  add_role_ids(diff)
                 end
               end
 
diff --git a/lib/sequencer/unit/import/ldap/users/sum.rb b/lib/sequencer/unit/import/ldap/users/total.rb
similarity index 91%
rename from lib/sequencer/unit/import/ldap/users/sum.rb
rename to lib/sequencer/unit/import/ldap/users/total.rb
index d9ffa7b2a..deb95cdcc 100644
--- a/lib/sequencer/unit/import/ldap/users/sum.rb
+++ b/lib/sequencer/unit/import/ldap/users/total.rb
@@ -5,7 +5,7 @@ class Sequencer
     module Import
       module Ldap
         module Users
-          class Sum < Sequencer::Unit::Base
+          class Total < Sequencer::Unit::Base
             include ::Sequencer::Unit::Import::Common::Model::Statistics::Mixin::EmptyDiff
 
             uses :ldap_config, :ldap_connection, :dry_run
@@ -13,14 +13,14 @@ class Sequencer
             def process
               state.provide(:statistics_diff) do
                 diff.merge(
-                  sum: sum
+                  total: total
                 )
               end
             end
 
             private
 
-            def sum
+            def total
               if !dry_run
                 result = Cache.get(cache_key)
               end