Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ensure_desired_properties always being enabled #1169

Merged
merged 18 commits into from
Feb 8, 2024
14 changes: 5 additions & 9 deletions azure-iot-device/azure/iot/device/iothub/abstract_clients.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,10 @@ def _replace_user_supplied_sastoken(self, sastoken_str: str) -> None:
raise ValueError("Provided SasToken is for a device")
if self._mqtt_pipeline.pipeline_configuration.device_id != vals["device_id"]:
raise ValueError("Provided SasToken does not match existing device id")
if vals["module_id"] != "" and self._mqtt_pipeline.pipeline_configuration.module_id != vals["module_id"]:
if (
vals["module_id"] != ""
and self._mqtt_pipeline.pipeline_configuration.module_id != vals["module_id"]
):
raise ValueError("Provided SasToken does not match existing module id")
if self._mqtt_pipeline.pipeline_configuration.hostname != vals["hostname"]:
raise ValueError("Provided SasToken does not match existing hostname")
Expand Down Expand Up @@ -433,9 +436,7 @@ def receive_method_request(self, method_name: Optional[str] = None) -> None:
pass

@abc.abstractmethod
def send_method_response(
self, method_response: MethodResponse
) -> None:
def send_method_response(self, method_response: MethodResponse) -> None:
pass

@abc.abstractmethod
Expand Down Expand Up @@ -607,7 +608,6 @@ def create_from_x509_certificate(
device_id=device_id, hostname=hostname, x509=x509, **config_kwargs
)
pipeline_configuration.blob_upload = True # Blob Upload is a feature on Device Clients
pipeline_configuration.ensure_desired_properties = True

# Pipeline setup
http_pipeline = pipeline.HTTPPipeline(pipeline_configuration)
Expand Down Expand Up @@ -680,7 +680,6 @@ def create_from_symmetric_key(
device_id=device_id, hostname=hostname, sastoken=sastoken, **config_kwargs
)
pipeline_configuration.blob_upload = True # Blob Upload is a feature on Device Clients
pipeline_configuration.ensure_desired_properties = True

# Pipeline setup
http_pipeline = pipeline.HTTPPipeline(pipeline_configuration)
Expand Down Expand Up @@ -844,8 +843,6 @@ def create_from_edge_environment(cls, **kwargs) -> Self:
server_verification_cert=server_verification_cert,
**config_kwargs,
)
pipeline_configuration.ensure_desired_properties = True

pipeline_configuration.method_invoke = (
True # Method Invoke is allowed on modules created from edge environment
)
Expand Down Expand Up @@ -912,7 +909,6 @@ def create_from_x509_certificate(
pipeline_configuration = pipeline.IoTHubPipelineConfig(
device_id=device_id, module_id=module_id, hostname=hostname, x509=x509, **config_kwargs
)
pipeline_configuration.ensure_desired_properties = True

# Pipeline setup
http_pipeline = pipeline.HTTPPipeline(pipeline_configuration)
Expand Down
2 changes: 1 addition & 1 deletion requirements_test.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
pytest
pytest < 8.0.0 # lazy_fixture currently broken in 8
pytest-mock
pytest-asyncio <= 0.16 # Can remove this once Python 3.6 support is dropped
pytest-testdox>=1.1.1
Expand Down
17 changes: 12 additions & 5 deletions tests/unit/iothub/shared_client_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,46 +165,50 @@ def test_product_info_option(
@pytest.mark.it(
"Sets the 'ensure_desired_properties' user option parameter on the PipelineConfig, if provided"
)
@pytest.mark.parametrize("edp_value", [True, False])
def test_ensure_desired_properties_option(
self,
option_test_required_patching,
client_create_method,
create_method_args,
mock_mqtt_pipeline_init,
mock_http_pipeline_init,
edp_value,
):

client_create_method(*create_method_args, ensure_desired_properties=True)
client_create_method(*create_method_args, ensure_desired_properties=edp_value)

# Get configuration object, and ensure it was used for both protocol pipelines
assert mock_mqtt_pipeline_init.call_count == 1
config = mock_mqtt_pipeline_init.call_args[0][0]
assert isinstance(config, IoTHubPipelineConfig)
assert config == mock_http_pipeline_init.call_args[0][0]

assert config.ensure_desired_properties is True
assert config.ensure_desired_properties is edp_value

@pytest.mark.it(
"Sets the 'websockets' user option parameter on the PipelineConfig, if provided"
)
@pytest.mark.parametrize("ws_value", [True, False])
def test_websockets_option(
self,
option_test_required_patching,
client_create_method,
create_method_args,
mock_mqtt_pipeline_init,
mock_http_pipeline_init,
ws_value,
):

client_create_method(*create_method_args, websockets=True)
client_create_method(*create_method_args, websockets=ws_value)

# Get configuration object, and ensure it was used for both protocol pipelines
assert mock_mqtt_pipeline_init.call_count == 1
config = mock_mqtt_pipeline_init.call_args[0][0]
assert isinstance(config, IoTHubPipelineConfig)
assert config == mock_http_pipeline_init.call_args[0][0]

assert config.websockets
assert config.websockets is ws_value

# TODO: Show that input in the wrong format is formatted to the correct one. This test exists
# in the IoTHubPipelineConfig object already, but we do not currently show that this is felt
Expand Down Expand Up @@ -320,15 +324,16 @@ def test_keep_alive_options(
@pytest.mark.it(
"Sets the 'auto_connect' user option parameter on the PipelineConfig, if provided"
)
@pytest.mark.parametrize("auto_connect_value", [True, False])
def test_auto_connect_option(
self,
option_test_required_patching,
client_create_method,
create_method_args,
mock_mqtt_pipeline_init,
mock_http_pipeline_init,
auto_connect_value,
):
auto_connect_value = False
client_create_method(*create_method_args, auto_connect=auto_connect_value)

# Get configuration object, and ensure it was used for both protocol pipelines
Expand All @@ -342,13 +347,15 @@ def test_auto_connect_option(
@pytest.mark.it(
"Sets the 'connection_retry' user option parameter on the PipelineConfig, if provided"
)
@pytest.mark.parametrize("connection_retry_value", [True, False])
def test_connection_retry_option(
self,
option_test_required_patching,
client_create_method,
create_method_args,
mock_mqtt_pipeline_init,
mock_http_pipeline_init,
connection_retry_value,
):
connection_retry_value = False
client_create_method(*create_method_args, connection_retry=connection_retry_value)
Expand Down
Loading