clean up too-many-branches / logic

1 files changed, 91 insertions, 56 deletions

diff --git a/roles/lib_zabbix/library/zbx_action.py b/roles/lib_zabbix/library/zbx_action.py
index c08bef4f7..2f9524556 100644
--- a/roles/lib_zabbix/library/zbx_action.py
+++ b/roles/lib_zabbix/library/zbx_action.py
@@ -81,6 +81,61 @@ def filter_differences(zabbix_filters, user_filters):
 
     return rval
 
+def opconditions_diff(zab_val, user_val):
+    ''' Report whether there are differences between opconditions on
+        zabbix and opconditions supplied by user '''
+
+    if len(zab_val) != len(user_val):
+        return True
+
+    for z_cond, u_cond in zip(zab_val, user_val):
+        if not all([str(u_cond[op_key]) == z_cond[op_key] for op_key in \
+                    ['conditiontype', 'operator', 'value']]):
+            return True
+
+    return False
+
+def opmessage_diff(zab_val, user_val):
+    ''' Report whether there are differences between opmessage on
+        zabbix and opmessage supplied by user '''
+
+    for op_msg_key, op_msg_val in user_val.items():
+        if zab_val[op_msg_key] != str(op_msg_val):
+            return True
+
+    return False
+
+def opmessage_grp_diff(zab_val, user_val):
+    ''' Report whether there are differences between opmessage_grp
+        on zabbix and opmessage_grp supplied by user '''
+
+    zab_grp_ids = set([ugrp['usrgrpid'] for ugrp in zab_val])
+    usr_grp_ids = set([ugrp['usrgrpid'] for ugrp in user_val])
+    if usr_grp_ids != zab_grp_ids:
+        return True
+
+    return False
+
+def opmessage_usr_diff(zab_val, user_val):
+    ''' Report whether there are differences between opmessage_usr
+        on zabbix and opmessage_usr supplied by user '''
+
+    zab_usr_ids = set([usr['usrid'] for usr in zab_val])
+    usr_ids = set([usr['usrid'] for usr in user_val])
+    if usr_ids != zab_usr_ids:
+        return True
+
+    return False
+
+def opcommand_diff(zab_op_cmd, usr_op_cmd):
+    ''' Check whether user-provided opcommand matches what's already
+        stored in Zabbix '''
+
+    for usr_op_cmd_key, usr_op_cmd_val in usr_op_cmd.items():
+        if zab_op_cmd[usr_op_cmd_key] != str(usr_op_cmd_val):
+            return True
+    return False
+
 def host_in_zabbix(zab_hosts, usr_host):
     ''' Check whether a particular user host is already in the
         Zabbix list of hosts '''
@@ -106,23 +161,11 @@ def hostlist_in_zabbix(zab_hosts, usr_hosts):
 
     return True
 
-def opcommand_diff(zab_op_cmd, usr_op_cmd):
-    ''' Check whether user-provided opcommand matches what's already
-        stored in Zabbix '''
-
-    for usr_op_cmd_key, usr_op_cmd_val in usr_op_cmd.items():
-        if zab_op_cmd[usr_op_cmd_key] != str(usr_op_cmd_val):
-            return True
-    return False
-
-# This logic is quite complex.  We are comparing two lists of dictionaries.
-# The outer for-loops allow us to descend down into both lists at the same time
-# and then walk over the key,val pairs of the incoming user dict's changes
-# or updates.  The if-statements are looking at different sub-object types and
-# comparing them.  The other suggestion on how to write this is to write a recursive
-# compare function but for the time constraints and for complexity I decided to go
-# this route.
-# pylint: disable=too-many-branches
+# We are comparing two lists of dictionaries (the one stored on zabbix and the
+# one the user is providing). For each type of operation, determine whether there
+# is a difference between what is stored on zabbix and what the user is providing.
+# If there is a difference, we take the user-provided data for what needs to
+# be stored/updated into zabbix.
 def operation_differences(zabbix_ops, user_ops):
     '''Determine the differences from user and zabbix for operations'''
 
@@ -132,49 +175,41 @@ def operation_differences(zabbix_ops, user_ops):
 
     rval = {}
     for zab, user in zip(zabbix_ops, user_ops):
-        for key, val in user.items():
-            if key == 'opconditions':
-                if len(zab[key]) != len(val):
-                    rval[key] = val
-                    break
-                for z_cond, u_cond in zip(zab[key], user[key]):
-                    if not all([str(u_cond[op_key]) == z_cond[op_key] for op_key in \
-                                ['conditiontype', 'operator', 'value']]):
-                        rval[key] = val
-                        break
-            elif key == 'opmessage':
-                # Verify each passed param matches
-                for op_msg_key, op_msg_val in val.items():
-                    if zab[key][op_msg_key] != str(op_msg_val):
-                        rval[key] = val
-                        break
-
-            elif key == 'opmessage_grp':
-                zab_grp_ids = set([ugrp['usrgrpid'] for ugrp in zab[key]])
-                usr_grp_ids = set([ugrp['usrgrpid'] for ugrp in val])
-                if usr_grp_ids != zab_grp_ids:
-                    rval[key] = val
-
-            elif key == 'opmessage_usr':
-                zab_usr_ids = set([usr['userid'] for usr in zab[key]])
-                usr_ids = set([usr['userid'] for usr in val])
-                if usr_ids != zab_usr_ids:
-                    rval[key] = val
-
-            elif key == 'opcommand':
-                if opcommand_diff(zab[key], val):
-                    rval[key] = val
-                    break
+        for oper in user.keys():
+            if oper == 'opconditions' and opconditions_diff(zab[oper], \
+                                                                user[oper]):
+                rval[oper] = user[oper]
+
+            elif oper == 'opmessage' and opmessage_diff(zab[oper], \
+                                                        user[oper]):
+                rval[oper] = user[oper]
+
+            elif oper == 'opmessage_grp' and opmessage_grp_diff(zab[oper], \
+                                                                user[oper]):
+                rval[oper] = user[oper]
+
+            elif oper == 'opmessage_usr' and opmessage_usr_diff(zab[oper], \
+                                                                user[oper]):
+                rval[oper] = user[oper]
+
+            elif oper == 'opcommand' and opcommand_diff(zab[oper], \
+                                                        user[oper]):
+                rval[oper] = user[oper]
 
             # opcommand_grp can be treated just like opcommand_hst
             # as opcommand_grp[] is just a list of groups
-            elif key == 'opcommand_hst' or key == 'opcommand_grp':
-                if not hostlist_in_zabbix(zab[key], val):
-                    rval[key] = val
-                    break
+            elif oper == 'opcommand_hst' or oper == 'opcommand_grp':
+                if not hostlist_in_zabbix(zab[oper], user[oper]):
+                    rval[oper] = user[oper]
+
+            # if it's any other type of operation than the ones tested above
+            # just do a direct compare
+            elif oper not in ['opconditions', 'opmessage', 'opmessage_grp',
+                              'opmessage_usr', 'opcommand', 'opcommand_hst',
+                              'opcommand_grp'] \
+                        and str(zab[oper]) != str(user[oper]):
+                rval[oper] = user[oper]
 
-            elif zab[key] != str(val):
-                rval[key] = val
     return rval
 
 def get_users(zapi, users):