[SEBA-407] Minor refactoring to clarify logic

Change-Id: I5067a658553cd529b48c473a2cbdb7458aed414a
diff --git a/xos/synchronizer/model_policies/model_policy_att_workflow_driver_serviceinstance.py b/xos/synchronizer/model_policies/model_policy_att_workflow_driver_serviceinstance.py
index 5464685..4e1aacb 100644
--- a/xos/synchronizer/model_policies/model_policy_att_workflow_driver_serviceinstance.py
+++ b/xos/synchronizer/model_policies/model_policy_att_workflow_driver_serviceinstance.py
@@ -39,38 +39,73 @@
     def handle_update(self, si):
         self.logger.debug("MODEL_POLICY: handle_update for AttWorkflowDriverServiceInstance %s " % (si.id), onu_state=si.onu_state, authentication_state=si.authentication_state)
 
-        # validating ONU
-        if si.onu_state == "AWAITING" or si.onu_state == "ENABLED":
-            # we validate the ONU state only if it is enabled or awaiting,
-            # if it's disabled it means someone has disabled it
-            self.validate_onu_state(si)
-        else:
-            # clean the status and verify that the ONU is actually disabled
-            si.status_message = "ONU has been disabled"
-            self.update_onu(si.serial_number, "DISABLED")
+        # Changing ONU state can change auth state
+        # Changing auth state can change DHCP state
+        # So need to process in this order
+        self.process_onu_state(si)
+        self.process_auth_state(si)
+        self.process_dhcp_state(si)
+
+        self.validate_states(si)
 
         # handling the subscriber status
+        # It's a combination of all the other states
         subscriber = self.get_subscriber(si.serial_number)
-
         if subscriber:
             self.update_subscriber(subscriber, si)
 
+        si.save_changed_fields()
+
+    def process_onu_state(self, si):
+        if si.onu_state == "AWAITING" or si.onu_state == "ENABLED":
+            [valid, message] = AttHelpers.validate_onu(self.logger, si)
+            si.status_message = message
+            if valid:
+                si.onu_state = "ENABLED"
+                self.update_onu(si.serial_number, "ENABLED")
+            else:
+                si.onu_state = "DISABLED"
+                self.update_onu(si.serial_number, "DISABLED")
+        else:
+            si.status_message = "ONU has been disabled"
+            self.update_onu(si.serial_number, "DISABLED")
+
+    def process_auth_state(self, si):
+        auth_msgs = {
+            "AWAITING": " - Awaiting Authentication",
+            "REQUESTED": " - Authentication requested",
+            "STARTED": " - Authentication started",
+            "APPROVED": " - Authentication succeeded",
+            "DENIED": " - Authentication denied"
+        }
+        if si.onu_state == "DISABLED":
+            si.authentication_state = "AWAITING"
+        else:
+            si.status_message += auth_msgs[si.authentication_state]
+
+    def process_dhcp_state(self, si):
         if si.authentication_state in ["AWAITING", "REQUESTED", "STARTED"]:
             si.ip_address = ""
             si.mac_address = ""
             si.dhcp_state = "AWAITING"
 
-        si.save_changed_fields()
+    # Make sure the object is in a legitimate state
+    # It should be after the above processing steps
+    # However this might still fail if an event has fired in the meantime
+    # Valid states:
+    # ONU       | Auth     | DHCP
+    # ===============================
+    # AWAITING  | AWAITING | AWAITING
+    # ENABLED   | *        | AWAITING
+    # ENABLED   | APPROVED | *
+    # DISABLED  | AWAITING | AWAITING
+    def validate_states(self, si):
+        if (si.onu_state == "AWAITING" or si.onu_state == "DISABLED") and si.authentication_state == "AWAITING" and si.dhcp_state == "AWAITING":
+            return
+        if si.onu_state == "ENABLED" and (si.authentication_state == "APPROVED" or si.dhcp_state == "AWAITING"):
+            return
+        self.logger.warning("MODEL_POLICY (validate_states): invalid state combination", onu_state=si.onu_state, auth_state=si.authentication_state, dhcp_state=si.dhcp_state)
 
-    def validate_onu_state(self, si):
-        [valid, message] = AttHelpers.validate_onu(self.logger, si)
-        si.status_message = message
-        if valid:
-            si.onu_state = "ENABLED"
-            self.update_onu(si.serial_number, "ENABLED")
-        else:
-            si.onu_state = "DISABLED"
-            self.update_onu(si.serial_number, "DISABLED")
 
     def update_onu(self, serial_number, admin_state):
         onu = [onu for onu in ONUDevice.objects.all() if onu.serial_number.lower() == serial_number.lower()][0]
@@ -121,21 +156,14 @@
 
     def update_subscriber(self, subscriber, si):
         cur_status = subscriber.status
-        if si.authentication_state == "AWAITING":
-            subscriber.status = "awaiting-auth"
-            si.status_message += " - Awaiting Authentication"
-        elif si.authentication_state == "REQUESTED":
-            subscriber.status = "awaiting-auth"
-            si.status_message += " - Authentication requested"
-        elif si.authentication_state == "STARTED":
-            subscriber.status = "awaiting-auth"
-            si.status_message += " - Authentication started"
-        elif si.authentication_state == "APPROVED":
-            subscriber.status = "enabled"
-            si.status_message += " - Authentication succeded"
-        elif si.authentication_state == "DENIED":
-            subscriber.status = "auth-failed"
-            si.status_message += " - Authentication denied"
+        # Don't change state if someone has disabled the subscriber
+        if subscriber.status != "disabled":
+            if si.authentication_state in ["AWAITING", "REQUESTED", "STARTED"]:
+                subscriber.status = "awaiting-auth"
+            elif si.authentication_state == "APPROVED":
+                subscriber.status = "enabled"
+            elif si.authentication_state == "DENIED":
+                subscriber.status = "auth-failed"
 
         # NOTE we save the subscriber only if:
         # - the status has changed