fix (azure-iot-device): PipelineNucleus connection state is now the single source of truth regarding connection (#1014)

* Removed the boolean `.connected` attribute being set on the `PipelineNucleus` by the `PipelineRootStage`
* `.connected` is now a property that is derived from the `connection_state` attribute, which is set by the `ConnectionStateStage`
* This is now the single source of truth for the connection state
* Added additional testing fixtures to allow for easy mocking of the connection state
* Note that changes made to the `ConnectionLockStage` are merely a stopgap until the entire stage can be removed

Author: cartertinney

azure-iot-device/azure/iot/device/common/pipeline/pipeline_nucleus.py

@@ -4,10 +4,26 @@
 # license information.
 # --------------------------------------------------------------------------
 
+from enum import Enum
+
 
 class PipelineNucleus(object):
     """Contains data and information shared across the pipeline"""
 
     def __init__(self, pipeline_configuration):
         self.pipeline_configuration = pipeline_configuration
-        self.connected = False
+        self.connection_state = ConnectionState.DISCONNECTED
+
+    @property
+    def connected(self):
+        # Only return True if fully connected
+        return self.connection_state is ConnectionState.CONNECTED
+
+
+class ConnectionState(Enum):
+    CONNECTED = "CONNECTED"  # Client is connected (as far as it knows)
+    DISCONNECTED = "DISCONNECTED"  # Client is disconnected
+    CONNECTING = "CONNECTING"  # Client is in the process of connecting
+    DISCONNECTING = "DISCONNECTING"  # Client is in the process of disconnecting
+    REAUTHORIZING = "REAUTHORIZING"  # Client is in the process of reauthorizing
+    # NOTE: Reauthorizing is the process of doing a disconnect, then a connect at transport level

azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_base.py

@@ -9,4 +9,6 @@
     arbitrary_op,
     fake_pipeline_thread,
     fake_non_pipeline_thread,
+    pipeline_connected_mock,
+    nucleus,
 )

azure-iot-device/tests/common/pipeline/conftest.py

@@ -8,6 +8,7 @@
 from azure.iot.device.common.pipeline import (
     pipeline_events_base,
     pipeline_ops_base,
+    pipeline_nucleus,
 )
 
 
@@ -33,6 +34,46 @@ def arbitrary_op(mocker):
     return op
 
 
+@pytest.fixture
+def pipeline_connected_mock(mocker):
+    """This mock can have it's return value altered by any test to indicate whether or not the
+    pipeline is connected (boolean).
+
+    Because this fixture is used by the nucleus fixture, and the nucleus is the single source of
+    truth for connection, changing this fixture's return value will change the connection state
+    of any other aspect of the pipeline (assuming it is using the nucleus fixture).
+
+    This has to be it's own fixture, because due to how PropertyMocks work, you can't access them
+    on an instance of an object like you can, say, the mocked settings on a PipelineConfiguration
+    """
+    p = mocker.PropertyMock()
+    return p
+
+
+@pytest.fixture
+def nucleus(mocker, pipeline_connected_mock):
+    """This fixture can be used to configure stages. Connection status can be mocked
+    via the above pipeline_connected_mock, but by default .connected will return a real value.
+    This nucleus will also come configured with a mocked pipeline configuration, which can be
+    overridden if necessary
+    """
+    # Need to use a mock for pipeline config because we don't know
+    # what type of config is being used since these are common
+    nucleus = pipeline_nucleus.PipelineNucleus(pipeline_configuration=mocker.MagicMock())
+
+    # By default, set the connected mock to return the real connected value
+    # (this can be overridden by changing the return value of pipeline_connected_mock)
+    def dynamic_return():
+        if not isinstance(pipeline_connected_mock.return_value, mocker.Mock):
+            return pipeline_connected_mock.return_value
+        return nucleus.connection_state is pipeline_nucleus.ConnectionState.CONNECTED
+
+    pipeline_connected_mock.side_effect = dynamic_return
+    type(nucleus).connected = pipeline_connected_mock
+
+    return nucleus
+
+
 @pytest.fixture
 def fake_pipeline_thread():
     """

azure-iot-device/tests/common/pipeline/fixtures.py

@@ -5,7 +5,7 @@
 # --------------------------------------------------------------------------
 import pytest
 import logging
-from azure.iot.device.common.pipeline.pipeline_nucleus import PipelineNucleus
+from azure.iot.device.common.pipeline.pipeline_nucleus import PipelineNucleus, ConnectionState
 
 
 logging.basicConfig(level=logging.DEBUG)
@@ -24,7 +24,39 @@ def test_pipeline_config(self, pipeline_config):
         nucleus = PipelineNucleus(pipeline_configuration=pipeline_config)
         assert nucleus.pipeline_configuration is pipeline_config
 
-    @pytest.mark.it("Instantiates with the 'connected' attribute set to False")
+    @pytest.mark.it("Instantiates with the 'connection_state' attribute set to DISCONNECTED")
     def test_connected(self, pipeline_config):
         nucleus = PipelineNucleus(pipeline_config)
-        assert nucleus.connected is False
+        assert nucleus.connection_state is ConnectionState.DISCONNECTED
+
+
+@pytest.mark.describe("PipelineNucleus - PROPERTY .connected")
+class TestPipelineNucleusPROPERTYConnected(object):
+    @pytest.fixture
+    def nucleus(self, mocker):
+        pl_cfg = mocker.MagicMock()
+        return PipelineNucleus(pl_cfg)
+
+    @pytest.mark.it("Is a read-only property")
+    def test_read_only(self, nucleus):
+        with pytest.raises(AttributeError):
+            nucleus.connected = False
+
+    @pytest.mark.it("Returns True if the '.connection_state' attribute is CONNECTED")
+    def test_connected(self, nucleus):
+        nucleus.connection_state = ConnectionState.CONNECTED
+        assert nucleus.connected
+
+    @pytest.mark.it("Returns False if the '.connection_state' attribute has any other value")
+    @pytest.mark.parametrize(
+        "state",
+        [
+            ConnectionState.DISCONNECTED,
+            ConnectionState.CONNECTING,
+            ConnectionState.DISCONNECTING,
+            ConnectionState.REAUTHORIZING,
+        ],
+    )
+    def test_not_connected(self, nucleus, state):
+        nucleus.connection_state = state
+        assert not nucleus.connected