[SEBA-241] Cleaning up and fixing behaviour

Change-Id: I2657c690a6b114613aa2aa434875f4f9ef4a7ee2
diff --git a/xos/synchronizer/model_policies/model_policy_att_workflow_driver_service.py b/xos/synchronizer/model_policies/model_policy_att_workflow_driver_service.py
index 0c9d978..f738c3a 100644
--- a/xos/synchronizer/model_policies/model_policy_att_workflow_driver_service.py
+++ b/xos/synchronizer/model_policies/model_policy_att_workflow_driver_service.py
@@ -25,8 +25,7 @@
 
         sis = AttWorkflowDriverServiceInstance.objects.all()
 
-        # TODO(smbaker): This is redudant with AttWorkflowDriverWhiteListEntry model policy, though etaining this does provide
-        # a handy way to trigger a full reexamination of the whitelist.
+        # TODO(teone): use the method defined in helpers.py
 
         whitelist = [x.serial_number.lower() for x in service.whitelist_entries.all()]
 
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 26b0367..dffd154 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
@@ -14,83 +14,90 @@
 # limitations under the License.
 
 
+
 from synchronizers.new_base.modelaccessor import RCORDSubscriber, ONUDevice, model_accessor
 from synchronizers.new_base.policy import Policy
 
+import os
+import sys
+
+sync_path = os.path.abspath(os.path.join(os.path.dirname(os.path.realpath(__file__)), ".."))
+sys.path.append(sync_path)
+
+from helpers import AttHelpers
+
 class DeferredException(Exception):
     pass
 
 class AttWorkflowDriverServiceInstancePolicy(Policy):
     model_name = "AttWorkflowDriverServiceInstance"
 
+    separator = " // "
+
     def handle_create(self, si):
         self.logger.debug("MODEL_POLICY: handle_create for AttWorkflowDriverServiceInstance %s " % si.id)
         self.handle_update(si)
 
     def handle_update(self, si):
+        self.logger.debug("MODEL_POLICY: handle_update for AttWorkflowDriverServiceInstance %s " % (si.id))
 
-        # TODO if si.onu_state = DISABLED set subscriber.status to need_auth
-        # TODO cleanup
+        # 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)
 
-        self.logger.debug("MODEL_POLICY: handle_update for AttWorkflowDriverServiceInstance %s, valid=%s " % (si.id, si.valid))
+        # handling the subscriber status
+        subscriber = self.get_subscriber(si.serial_number)
 
-        # Check to make sure the object has been synced. This is to cover a race condition where the model_policy
-        # runs, is interrupted by the sync step, the sync step completes, and then the model policy ends up saving
-        # a policed_timestamp that is later the updated timestamp set by the sync_step.
-        if (si.backend_code!=1):
-            raise DeferredException("MODEL_POLICY: AttWorkflowDriverServiceInstance %s has not been synced yet" % si.id)
+        if subscriber:
+            self.update_subscriber(subscriber, si)
 
-        # waiting for Whitelist validation
-        if not hasattr(si, 'valid') or si.valid is "awaiting":
-            raise DeferredException("MODEL_POLICY: deferring handle_update for AttWorkflowDriverServiceInstance %s as not validated yet" % si.id)
+        si.save()
 
-        # disabling ONU
-        if si.valid == "invalid":
-            self.logger.debug("MODEL_POLICY: disabling ONUDevice [%s] for AttWorkflowDriverServiceInstance %s" % (si.serial_number, si.id))
-            onu = ONUDevice.objects.get(serial_number=si.serial_number)
-            onu.admin_state = "DISABLED"
-            onu.save(always_update_timestamp=True)
-            return
-        if si.valid == "valid":
+    def validate_onu_state(self, si):
+        [valid, message] = AttHelpers.validate_onu(si)
+        si.status_message += self.separator + 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")
 
-            # reactivating the ONUDevice
-            try:
-                onu = ONUDevice.objects.get(serial_number=si.serial_number)
-            except IndexError:
-                raise Exception("MODEL_POLICY: cannot find ONUDevice [%s] for AttWorkflowDriverServiceInstance %s" % (si.serial_number, si.id))
-            if onu.admin_state == "DISABLED":
-                self.logger.debug("MODEL_POLICY: enabling ONUDevice [%s] for AttWorkflowDriverServiceInstance %s" % (si.serial_number, si.id))
-                onu.admin_state = "ENABLED"
-                onu.save(always_update_timestamp=True)
+    def update_onu(self, serial_number, admin_state):
+        self.logger.debug("MODEL_POLICY: setting ONUDevice [%s] admin_state to %s" % (serial_number, admin_state))
+        onu = ONUDevice.objects.get(serial_number=serial_number)
+        onu.admin_state = admin_state
+        onu.save(always_update_timestamp=True)
 
-            # handling the subscriber status
+    def get_subscriber(self, serial_number):
+        try:
+            return [s for s in RCORDSubscriber.objects.all() if s.onu_device.lower() == serial_number.lower()][0]
+        except IndexError:
+            # If the subscriber doesn't exist we don't do anything
+            self.logger.debug("MODEL_POLICY: subscriber does not exists for this SI, doing nothing", onu_device=serial_number)
+            return None
 
-            subscriber = None
-            try:
-                subscriber = [s for s in RCORDSubscriber.objects.all() if s.onu_device.lower() == si.serial_number.lower()][0]
-            except IndexError:
-                # we just want to find out if it exists or not
-                pass
+    def update_subscriber(self, subscriber, si):
+        if si.authentication_state == "AWAITING":
+            subscriber.status = "awaiting-auth"
+            si.status_message += self.separator + "Awaiting Authentication"
+        elif si.authentication_state == "REQUESTED":
+            subscriber.status = "awaiting-auth"
+            si.status_message += self.separator + "Authentication requested"
+        elif si.authentication_state == "STARTED":
+            subscriber.status = "awaiting-auth"
+            si.status_message += self.separator + "Authentication started"
+        elif si.authentication_state == "APPROVED":
+            subscriber.status = "enabled"
+            si.status_message += self.separator + "Authentication succeded"
+        elif si.authentication_state == "DENIED":
+            subscriber.status = "auth-failed"
+            si.status_message += self.separator + "Authentication denied"
+        self.logger.debug("MODEL_POLICY: handling subscriber", onu_device=subscriber.onu_device, authentication_state=si.authentication_state, subscriber_status=subscriber.status)
 
-            if subscriber:
-                # if the subscriber is there and authentication is complete, update its state
-                self.logger.debug("MODEL_POLICY: handling subscriber", onu_device=si.serial_number, authentication_state=si.authentication_state, onu_state=si.onu_state)
-                if si.onu_state == "DISABLED":
-                    # NOTE do not mess with onu.admin_state as that triggered this condition
-                    subscriber.status = "awaiting-auth"
-                elif si.authentication_state == "STARTED":
-                    subscriber.status = "awaiting-auth"
-                elif si.authentication_state == "REQUESTED":
-                    subscriber.status = "awaiting-auth"
-                elif si.authentication_state == "APPROVED":
-                    subscriber.status = "enabled"
-                elif si.authentication_state == "DENIED":
-                    subscriber.status = "auth-failed"
-
-                subscriber.save(always_update_timestamp=True)
-            # if subscriber does not exist
-            else:
-                self.logger.warn("MODEL_POLICY: subscriber does not exists for this SI, doing nothing")
+        subscriber.save(always_update_timestamp=True)
 
     def handle_delete(self, si):
         pass
diff --git a/xos/synchronizer/model_policies/model_policy_att_workflow_driver_whitelistentry.py b/xos/synchronizer/model_policies/model_policy_att_workflow_driver_whitelistentry.py
index c788d23..5cb4c23 100644
--- a/xos/synchronizer/model_policies/model_policy_att_workflow_driver_whitelistentry.py
+++ b/xos/synchronizer/model_policies/model_policy_att_workflow_driver_whitelistentry.py
@@ -31,6 +31,8 @@
         #     serial_number = whitelist.serial_number,
         #     owner_id = whitelist.owner.id)
 
+        # TODO(teone): use the method defined in helpers.py
+
         sis = AttWorkflowDriverServiceInstance.objects.all()
 
         for si in sis:
diff --git a/xos/synchronizer/model_policies/test_model_policy_att_workflow_driver_serviceinstance.py b/xos/synchronizer/model_policies/test_model_policy_att_workflow_driver_serviceinstance.py
index c47738d..18c8174 100644
--- a/xos/synchronizer/model_policies/test_model_policy_att_workflow_driver_serviceinstance.py
+++ b/xos/synchronizer/model_policies/test_model_policy_att_workflow_driver_serviceinstance.py
@@ -64,165 +64,146 @@
         # tags. Ideally, this wouldn't happen, but it does. So make sure we reset the world.
         model_accessor.reset_all_object_stores()
 
+
         self.policy = AttWorkflowDriverServiceInstancePolicy()
         self.si = AttWorkflowDriverServiceInstance()
         self.si.owner = AttWorkflowDriverService()
+        self.si.serial_number = "BRCM1234"
 
     def tearDown(self):
         sys.path = self.sys_path_save
-        self.si = None
-
-    def test_not_synced(self):
-        self.si.valid = "awaiting"
-        self.si.backend_code = 0
-
-        with patch.object(RCORDSubscriber, "save") as subscriber_save, \
-            patch.object(ONUDevice, "save") as onu_save:
-
-            with self.assertRaises(Exception) as e:
-               self.policy.handle_update(self.si)
-
-            self.assertIn("has not been synced yet", e.exception.message)
-
-    def test_defer_update(self):
-        self.si.valid = "awaiting"
-        self.si.backend_code = 1
-
-        with patch.object(RCORDSubscriber, "save") as subscriber_save, \
-            patch.object(ONUDevice, "save") as onu_save:
-
-            with self.assertRaises(Exception) as e:
-                self.policy.handle_update(self.si)
-
-            self.assertEqual(e.exception.message, "MODEL_POLICY: deferring handle_update for AttWorkflowDriverServiceInstance 98052 as not validated yet")
-            subscriber_save.assert_not_called()
-            onu_save.assert_not_called()
-
-    def test_disable_onu(self):
-        self.si.valid = "invalid"
-        self.si.serial_number = "BRCM1234"
-        self.si.backend_code = 1
-        self.si.onu_state = "ENABLED"
-
-        onu = ONUDevice(
-            serial_number=self.si.serial_number
-        )
-
-        with patch.object(ONUDevice.objects, "get_items") as onu_objects, \
-                patch.object(RCORDSubscriber, "save") as subscriber_save, \
-                patch.object(ONUDevice, "save") as onu_save:
-
-            onu_objects.return_value = [onu]
-
-            self.policy.handle_update(self.si)
-            subscriber_save.assert_not_called()
-            self.assertEqual(onu.admin_state, "DISABLED")
-            onu_save.assert_called()
 
     def test_enable_onu(self):
-        self.si.valid = "valid"
-        self.si.serial_number = "BRCM1234"
-        self.si.c_tag = None
-        self.si.backend_code = 1
-        self.si.onu_state = "ENABLED"
+        from helpers import AttHelpers
+        with patch.object(AttHelpers, "validate_onu") as validate_onu, \
+            patch.object(self.policy, "update_onu") as update_onu, \
+            patch.object(self.si, "save") as save_si:
+            validate_onu.return_value = [True, "valid onu"]
 
-        onu = ONUDevice(
-            serial_number=self.si.serial_number,
-            admin_state="DISABLED"
-        )
+            self.policy.validate_onu_state(self.si)
 
-        subscriber = RCORDSubscriber(
-            onu_device=self.si.serial_number,
-            status='pre-provisioned'
-        )
+            update_onu.assert_called_once()
+            update_onu.assert_called_with("BRCM1234", "ENABLED")
 
-        with patch.object(ONUDevice.objects, "get_items") as onu_objects, \
-                patch.object(RCORDSubscriber.objects, "get_items") as subscriber_objects, \
-                patch.object(ONUDevice, "save") as onu_save:
+            self.assertIn("valid onu", self.si.status_message)
 
-            onu_objects.return_value = [onu]
-            subscriber_objects.return_value = [subscriber]
+    def test_disable_onu(self):
+        from helpers import AttHelpers
+        with patch.object(AttHelpers, "validate_onu") as validate_onu, \
+                patch.object(self.policy, "update_onu") as update_onu, \
+                patch.object(self.si, "save") as save_si:
+            validate_onu.return_value = [False, "invalid onu"]
 
+            self.policy.validate_onu_state(self.si)
+
+            update_onu.assert_called_once()
+            update_onu.assert_called_with("BRCM1234", "DISABLED")
+
+            self.assertIn("invalid onu", self.si.status_message)
+
+    def test_handle_update_validate_onu(self):
+        """
+        Testing that handle_update calls validate_onu with the correct parameters
+        when necessary
+        """
+        with patch.object(self.policy, "validate_onu_state") as validate_onu_state, \
+            patch.object(self.policy, "update_onu") as update_onu, \
+            patch.object(self.policy, "get_subscriber") as get_subscriber:
+            update_onu.return_value = None
+            get_subscriber.return_value = None
+
+            self.si.onu_state = "AWAITING"
             self.policy.handle_update(self.si)
-            self.assertEqual(onu.admin_state, "ENABLED")
-            onu_save.assert_called()
+            validate_onu_state.assert_called_with(self.si)
 
-    def test_do_not_create_subscriber(self):
-        self.si.valid = "valid"
-        self.si.backend_code = 1
-        self.si.serial_number = "BRCM1234"
-        self.si.authentication_state = "DENIEND"
-        self.si.onu_state = "ENABLED"
-
-        onu = ONUDevice(
-            serial_number=self.si.serial_number,
-            admin_state="DISABLED"
-        )
-        
-        with patch.object(ONUDevice.objects, "get_items") as onu_objects, \
-                patch.object(RCORDSubscriber, "save", autospec=True) as subscriber_save, \
-                patch.object(ONUDevice, "save") as onu_save:
-
-            onu_objects.return_value = [onu]
-
+            self.si.onu_state = "ENABLED"
             self.policy.handle_update(self.si)
+            validate_onu_state.assert_called_with(self.si)
 
-            self.assertEqual(onu.admin_state, "ENABLED")
-            onu_save.assert_called()
-            self.assertEqual(subscriber_save.call_count, 0)
+            self.si.onu_state = "DISABLED"
+            self.policy.handle_update(self.si)
+            self.assertEqual(validate_onu_state.call_count, 2)
 
-    def test_subscriber_awaiting_status_onu_state_disabled(self):
-        self.si.valid = "valid"
-        self.si.backend_code = 1
-        self.si.serial_number = "BRCM1234"
+    def test_get_subscriber(self):
+
+        sub = RCORDSubscriber(
+            onu_device="BRCM1234"
+        )
+
+        with patch.object(RCORDSubscriber.objects, "get_items") as get_subscribers:
+            get_subscribers.return_value = [sub]
+
+            res = self.policy.get_subscriber("BRCM1234")
+            self.assertEqual(res, sub)
+
+            res = self.policy.get_subscriber("brcm1234")
+            self.assertEqual(res, sub)
+
+            res = self.policy.get_subscriber("foo")
+            self.assertEqual(res, None)
+
+    def test_update_subscriber(self):
+
+        sub = RCORDSubscriber(
+            onu_device="BRCM1234"
+        )
+
+        self.si.status_message = "some content"
+
+        with patch.object(sub, "save") as sub_save:
+            self.si.authentication_state = "AWAITING"
+            self.policy.update_subscriber(sub, self.si)
+            self.assertEqual(sub.status, "awaiting-auth")
+            self.assertIn("Awaiting Authentication", self.si.status_message)
+            sub_save.assert_called()
+            sub_save.reset_mock()
+
+            self.si.authentication_state = "REQUESTED"
+            self.policy.update_subscriber(sub, self.si)
+            self.assertEqual(sub.status, "awaiting-auth")
+            self.assertIn("Authentication requested", self.si.status_message)
+            sub_save.assert_called()
+            sub_save.reset_mock()
+
+            self.si.authentication_state = "STARTED"
+            self.policy.update_subscriber(sub, self.si)
+            self.assertEqual(sub.status, "awaiting-auth")
+            self.assertIn("Authentication started", self.si.status_message)
+            sub_save.assert_called()
+            sub_save.reset_mock()
+
+            self.si.authentication_state = "APPROVED"
+            self.policy.update_subscriber(sub, self.si)
+            self.assertEqual(sub.status, "enabled")
+            self.assertIn("Authentication succeded", self.si.status_message)
+            sub_save.assert_called()
+            sub_save.reset_mock()
+
+            self.si.authentication_state = "DENIED"
+            self.policy.update_subscriber(sub, self.si)
+            self.assertEqual(sub.status, "auth-failed")
+            self.assertIn("Authentication denied", self.si.status_message)
+            sub_save.assert_called()
+            sub_save.reset_mock()
+
+    def test_handle_update_subscriber(self):
         self.si.onu_state = "DISABLED"
 
-        onu = ONUDevice(
-            serial_number=self.si.serial_number,
-            admin_state="DISABLED"
+        sub = RCORDSubscriber(
+            onu_device="BRCM1234"
         )
 
-        subscriber = RCORDSubscriber(
-            onu_device=self.si.serial_number,
-            status='enabled'
-        )
+        with patch.object(self.policy, "get_subscriber") as get_subscriber, \
+            patch.object(self.policy, "update_subscriber") as update_subscriber:
 
-        with patch.object(ONUDevice.objects, "get_items") as onu_objects, \
-                patch.object(RCORDSubscriber.objects, "get_items") as subscriber_objects, \
-                patch.object(RCORDSubscriber, "save") as subscriber_save:
-            onu_objects.return_value = [onu]
-            subscriber_objects.return_value = [subscriber]
-
+            get_subscriber.return_value = None
             self.policy.handle_update(self.si)
-            self.assertEqual(subscriber.status, "awaiting-auth")
-            subscriber_save.assert_called()
+            self.assertEqual(update_subscriber.call_count, 0)
 
-    def test_subscriber_enable_status_auth_state_approved(self):
-        self.si.valid = "valid"
-        self.si.backend_code = 1
-        self.si.serial_number = "brcm1234"
-        self.si.onu_state = "ENABLED"
-        self.si.authentication_state = "APPROVED"
-
-        onu = ONUDevice(
-            serial_number=self.si.serial_number,
-            admin_state="ENABLED"
-        )
-
-        subscriber = RCORDSubscriber(
-            onu_device="BRCM1234",
-            status='awaiting-auth'
-        )
-
-        with patch.object(ONUDevice.objects, "get_items") as onu_objects, \
-                patch.object(RCORDSubscriber.objects, "get_items") as subscriber_objects, \
-                patch.object(RCORDSubscriber, "save") as subscriber_save:
-            onu_objects.return_value = [onu]
-            subscriber_objects.return_value = [subscriber]
-
+            get_subscriber.return_value = sub
             self.policy.handle_update(self.si)
-            self.assertEqual(subscriber.status, "enabled")
-            subscriber_save.assert_called()
+            update_subscriber.assert_called_with(sub, self.si)
+
 
 if __name__ == '__main__':
     unittest.main()