[SEBA-407] Minor refactoring to clarify logic
Change-Id: I5067a658553cd529b48c473a2cbdb7458aed414a
diff --git a/VERSION b/VERSION
index 7fdc1c8..9374c13 100644
--- a/VERSION
+++ b/VERSION
@@ -1,2 +1,2 @@
-1.0.13
+1.0.14
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
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 3b1b0df..1abc7c8 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
@@ -98,7 +98,7 @@
patch.object(self.si, "save") as save_si:
validate_onu.return_value = [True, "valid onu"]
- self.policy.validate_onu_state(self.si)
+ self.policy.process_onu_state(self.si)
update_onu.assert_called_once()
update_onu.assert_called_with("BRCM1234", "ENABLED")
@@ -112,7 +112,7 @@
patch.object(self.si, "save") as save_si:
validate_onu.return_value = [False, "invalid onu"]
- self.policy.validate_onu_state(self.si)
+ self.policy.process_onu_state(self.si)
update_onu.assert_called_once()
update_onu.assert_called_with("BRCM1234", "DISABLED")
@@ -124,7 +124,7 @@
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, \
+ with patch.object(self.policy, "process_onu_state") as process_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
@@ -132,15 +132,16 @@
self.si.onu_state = "AWAITING"
self.policy.handle_update(self.si)
- validate_onu_state.assert_called_with(self.si)
+ process_onu_state.assert_called_with(self.si)
self.si.onu_state = "ENABLED"
self.policy.handle_update(self.si)
- validate_onu_state.assert_called_with(self.si)
+ process_onu_state.assert_called_with(self.si)
self.si.onu_state = "DISABLED"
self.policy.handle_update(self.si)
- self.assertEqual(validate_onu_state.call_count, 2)
+ process_onu_state.assert_called_with(self.si)
+
def test_get_subscriber(self):
@@ -172,7 +173,6 @@
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()
sub.status = None
@@ -180,7 +180,6 @@
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()
sub.status = None
@@ -188,7 +187,6 @@
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()
sub.status = None
@@ -196,7 +194,6 @@
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()
sub.status = None
@@ -204,7 +201,6 @@
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()
sub.status = None
diff --git a/xos/synchronizer/models/att-workflow-driver.xproto b/xos/synchronizer/models/att-workflow-driver.xproto
index 7270c75..7ebc390 100644
--- a/xos/synchronizer/models/att-workflow-driver.xproto
+++ b/xos/synchronizer/models/att-workflow-driver.xproto
@@ -11,14 +11,14 @@
option verbose_name = "AttWorkflowDriver Service Instance";
required string serial_number = 2 [max_length = 254, db_index = False, tosca_key=True, unique = True];
- required string authentication_state = 3 [default = "AWAITING", choices = "(('AWAITING', 'Awaiting'), ('STARTED', 'Started'), ('REQUESTED', 'Requested'), ('APPROVED', 'Approved'), ('DENIED', 'Denied'), )", max_length = 50, db_index = False];
+ required string authentication_state = 3 [default = "AWAITING", choices = "(('AWAITING', 'Awaiting'), ('STARTED', 'Started'), ('REQUESTED', 'Requested'), ('APPROVED', 'Approved'), ('DENIED', 'Denied'), )", max_length = 50, db_index = False, feedback_state = True];
required string of_dpid = 4 [max_length = 254, db_index = False];
required int32 uni_port_id = 5 [db_index = False];
- required string onu_state = 6 [max_length = 254, db_index = False, default = "AWAITING", choices = "(('AWAITING', 'Awaiting'), ('ENABLED', 'Enabled'), ('DISABLED', 'Disabled'))"];
+ required string onu_state = 6 [max_length = 254, db_index = False, default = "AWAITING", choices = "(('AWAITING', 'Awaiting'), ('ENABLED', 'Enabled'), ('DISABLED', 'Disabled'))", feedback_state = True];
optional string status_message = 7 [max_length = 254, db_index = False, default = ""];
- required string dhcp_state = 8 [max_length = 254, db_index = False, default = "AWAITING", choices = "(('AWAITING', 'Awaiting'), ('DHCPDISCOVER', 'DHCPDISCOVER'), ('DHCPACK', 'DHCPACK'), ('DHCPREQUEST', 'DHCPREQUEST'))"];
- optional string ip_address = 9 [max_length = 20, db_index = False];
- optional string mac_address = 10 [max_length = 20, db_index = False];
+ required string dhcp_state = 8 [max_length = 254, db_index = False, default = "AWAITING", choices = "(('AWAITING', 'Awaiting'), ('DHCPDISCOVER', 'DHCPDISCOVER'), ('DHCPACK', 'DHCPACK'), ('DHCPREQUEST', 'DHCPREQUEST'))", feedback_state = True];
+ optional string ip_address = 9 [max_length = 20, db_index = False, feedback_state = True];
+ optional string mac_address = 10 [max_length = 20, db_index = False, feedback_state = True];
}
message AttWorkflowDriverWhiteListEntry (XOSBase) {