From 71ed5682d7b0d747428b5e8d42e6a2aad3b96512 Mon Sep 17 00:00:00 2001 From: Victoria Litvinova Date: Wed, 24 Feb 2021 15:39:10 -0800 Subject: [PATCH 1/4] Use if_none_match instead of replace --- azext_iot/digitaltwins/commands_twins.py | 8 ++++---- azext_iot/digitaltwins/params.py | 6 +++--- azext_iot/digitaltwins/providers/twin.py | 8 ++++---- .../digitaltwins/test_dt_twin_lifecycle_int.py | 6 +++--- .../tests/digitaltwins/test_dt_twins_unit.py | 16 ++++++++-------- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/azext_iot/digitaltwins/commands_twins.py b/azext_iot/digitaltwins/commands_twins.py index fb4570753..b219acd96 100644 --- a/azext_iot/digitaltwins/commands_twins.py +++ b/azext_iot/digitaltwins/commands_twins.py @@ -22,13 +22,13 @@ def create_twin( name_or_hostname, twin_id, model_id, - replace=False, + if_none_match=False, properties=None, resource_group_name=None ): twin_provider = TwinProvider(cmd=cmd, name=name_or_hostname, rg=resource_group_name) return twin_provider.create( - twin_id=twin_id, model_id=model_id, replace=replace, properties=properties + twin_id=twin_id, model_id=model_id, if_none_match=if_none_match, properties=properties ) @@ -54,7 +54,7 @@ def create_relationship( target_twin_id, relationship_id, relationship, - replace=False, + if_none_match=False, properties=None, resource_group_name=None, ): @@ -64,7 +64,7 @@ def create_relationship( target_twin_id=target_twin_id, relationship_id=relationship_id, relationship=relationship, - replace=replace, + if_none_match=if_none_match, properties=properties, ) diff --git a/azext_iot/digitaltwins/params.py b/azext_iot/digitaltwins/params.py index e54ca7c11..c9aa39b8f 100644 --- a/azext_iot/digitaltwins/params.py +++ b/azext_iot/digitaltwins/params.py @@ -287,9 +287,9 @@ def load_digitaltwins_arguments(self, _): help="The path to the DTDL component.", ) context.argument( - "replace", - options_list=["--replace"], - help="Indicates the operation should replace an existing twin if it exists." + "if_none_match", + options_list=["--if-none-match"], + help="Indicates the create operation should fail an existing twin if it exists." ) with self.argument_context("dt twin create") as context: diff --git a/azext_iot/digitaltwins/providers/twin.py b/azext_iot/digitaltwins/providers/twin.py index 5dada72d8..7bf8c06d1 100644 --- a/azext_iot/digitaltwins/providers/twin.py +++ b/azext_iot/digitaltwins/providers/twin.py @@ -57,7 +57,7 @@ def invoke_query(self, query, show_cost): return query_result - def create(self, twin_id, model_id, replace=False, properties=None): + def create(self, twin_id, model_id, if_none_match=False, properties=None): twin_request = { "$dtId": twin_id, "$metadata": {"$model": model_id}, @@ -72,7 +72,7 @@ def create(self, twin_id, model_id, replace=False, properties=None): logger.info("Twin payload %s", json.dumps(twin_request)) try: - options = TwinOptions(if_none_match=(None if replace else "*")) + options = TwinOptions(if_none_match=("*" if if_none_match else None)) return self.twins_sdk.add(id=twin_id, twin=twin_request, digital_twins_add_options=options) except ErrorResponseException as e: raise CLIError(unpack_msrest_error(e)) @@ -117,7 +117,7 @@ def add_relationship( target_twin_id, relationship_id, relationship, - replace=False, + if_none_match=False, properties=None, ): relationship_request = { @@ -133,7 +133,7 @@ def add_relationship( logger.info("Relationship payload %s", json.dumps(relationship_request)) try: - options = TwinOptions(if_none_match=(None if replace else "*")) + options = TwinOptions(if_none_match=("*" if if_none_match else None)) return self.twins_sdk.add_relationship( id=twin_id, relationship_id=relationship_id, diff --git a/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py b/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py index defe13cf9..4484aac78 100644 --- a/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py +++ b/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py @@ -112,7 +112,7 @@ def test_dt_twin(self): ) replaced_room_twin = self.cmd( - "dt twin create -n {} -g {} --dtmi {} --twin-id {} --replace --properties '{}'".format( + "dt twin create -n {} -g {} --dtmi {} --twin-id {} --properties '{}'".format( instance_name, self.rg, room_dtmi, @@ -129,9 +129,9 @@ def test_dt_twin(self): component_name=thermostat_component_id, ) - # new twin cannot be created with same twin_id if replace not provided + # new twin cannot be created with same twin_id if if-none-match provided self.cmd( - "dt twin create -n {} -g {} --dtmi {} --twin-id {} --properties '{}'".format( + "dt twin create -n {} -g {} --dtmi {} --twin-id {} --if-none-match --properties '{}'".format( instance_name, self.rg, room_dtmi, diff --git a/azext_iot/tests/digitaltwins/test_dt_twins_unit.py b/azext_iot/tests/digitaltwins/test_dt_twins_unit.py index e337f3b67..eb6141e97 100644 --- a/azext_iot/tests/digitaltwins/test_dt_twins_unit.py +++ b/azext_iot/tests/digitaltwins/test_dt_twins_unit.py @@ -247,7 +247,7 @@ def service_client(self, mocked_response, start_twin_response): yield mocked_response @pytest.mark.parametrize( - "replace, properties, resource_group_name", + "if_none_match, properties, resource_group_name", [ (False, None, None), (True, None, None), @@ -256,13 +256,13 @@ def service_client(self, mocked_response, start_twin_response): (False, generic_patch_2, None) ] ) - def test_create_twin(self, fixture_cmd, service_client, replace, properties, resource_group_name): + def test_create_twin(self, fixture_cmd, service_client, if_none_match, properties, resource_group_name): result = subject.create_twin( cmd=fixture_cmd, name_or_hostname=hostname, twin_id=twin_id, model_id=model_id, - replace=replace, + if_none_match=if_none_match, properties=properties, resource_group_name=resource_group_name ) @@ -277,7 +277,7 @@ def test_create_twin(self, fixture_cmd, service_client, replace, properties, res assert twin_request_body["$dtId"] == twin_id assert twin_request_body["$metadata"]["$model"] == model_id - if not replace: + if if_none_match: assert twin_request.headers["If-None-Match"] == "*" if properties: @@ -557,7 +557,7 @@ def service_client(self, mocked_response, start_twin_response): yield mocked_response @pytest.mark.parametrize( - "relationship, replace, properties, resource_group_name", + "relationship, if_none_match, properties, resource_group_name", [ ("contains", False, None, None), ("", False, None, None), @@ -567,7 +567,7 @@ def service_client(self, mocked_response, start_twin_response): ("contains", False, None, resource_group) ] ) - def test_create_relationship(self, fixture_cmd, service_client, relationship, replace, properties, resource_group_name): + def test_create_relationship(self, fixture_cmd, service_client, relationship, if_none_match, properties, resource_group_name): result = subject.create_relationship( cmd=fixture_cmd, name_or_hostname=hostname, @@ -575,7 +575,7 @@ def test_create_relationship(self, fixture_cmd, service_client, relationship, re target_twin_id=target_twin_id, relationship_id=relationship_id, relationship=relationship, - replace=replace, + if_none_match=if_none_match, properties=properties, resource_group_name=resource_group_name ) @@ -591,7 +591,7 @@ def test_create_relationship(self, fixture_cmd, service_client, relationship, re assert result_request_body["$targetId"] == target_twin_id assert result_request_body["$relationshipName"] == relationship - if not replace: + if if_none_match: assert put_request.headers["If-None-Match"] == "*" if properties: From 1fdffdff2daeebd02929a5e71f60a26c7f8c805f Mon Sep 17 00:00:00 2001 From: Victoria Litvinova Date: Wed, 24 Feb 2021 16:06:40 -0800 Subject: [PATCH 2/4] pylint error fix --- azext_iot/operations/hub.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azext_iot/operations/hub.py b/azext_iot/operations/hub.py index b85a8022f..22ec18854 100644 --- a/azext_iot/operations/hub.py +++ b/azext_iot/operations/hub.py @@ -2223,7 +2223,7 @@ def http_wrap(target, device_id, generator): max_runs=msg_count, return_handle=True, ) - while True and op.is_alive(): + while op.is_alive(): _handle_c2d_msg(target, device_id, receive_settle) sleep(SIM_RECEIVE_SLEEP_SEC) From 931cfeafa4c61a2e9878b083c860e9738fce4571 Mon Sep 17 00:00:00 2001 From: Victoria Litvinova Date: Wed, 24 Feb 2021 16:09:00 -0800 Subject: [PATCH 3/4] update help messaage --- azext_iot/digitaltwins/params.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azext_iot/digitaltwins/params.py b/azext_iot/digitaltwins/params.py index c9aa39b8f..96b695655 100644 --- a/azext_iot/digitaltwins/params.py +++ b/azext_iot/digitaltwins/params.py @@ -289,7 +289,7 @@ def load_digitaltwins_arguments(self, _): context.argument( "if_none_match", options_list=["--if-none-match"], - help="Indicates the create operation should fail an existing twin if it exists." + help="Indicates the create operation should fail if an existing twin with the same id exists." ) with self.argument_context("dt twin create") as context: From 10e30f1660b70092a4533641468cb18001ed3d91 Mon Sep 17 00:00:00 2001 From: Victoria Litvinova Date: Thu, 25 Feb 2021 11:20:45 -0800 Subject: [PATCH 4/4] update help and params, add etags to it --- azext_iot/digitaltwins/_help.py | 28 +++++ azext_iot/digitaltwins/params.py | 3 +- .../test_dt_twin_lifecycle_int.py | 104 ++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) diff --git a/azext_iot/digitaltwins/_help.py b/azext_iot/digitaltwins/_help.py index a581f8d0c..8f73c472e 100644 --- a/azext_iot/digitaltwins/_help.py +++ b/azext_iot/digitaltwins/_help.py @@ -442,6 +442,11 @@ def load_digitaltwins_help(): az dt twin create -n {instance_or_hostname} --dtmi "dtmi:com:example:Room;1" --twin-id {twin_id} + - name: Create a digital twin from an existing (prior-created) model with if-none-match tag. + text: > + az dt twin create -n {instance_or_hostname} --dtmi "dtmi:com:example:Room;1" + --twin-id {twin_id} --if-none-match + - name: Create a digital twin from an existing (prior-created) model. Instantiate with property values. text: > az dt twin create -n {instance_or_hostname} --dtmi "dtmi:com:example:DeviceInformation;1" @@ -478,6 +483,11 @@ def load_digitaltwins_help(): az dt twin update -n {instance_or_hostname} --twin-id {twin_id} --json-patch '{"op":"replace", "path":"/Temperature", "value": 20.5}' + - name: Update a digital twin via JSON patch specification and using etag. + text: > + az dt twin update -n {instance_or_hostname} --twin-id {twin_id} --etag {etag} + --json-patch '{"op":"replace", "path":"/Temperature", "value": 20.5}' + - name: Update a digital twin via JSON patch specification. text: > az dt twin update -n {instance_or_hostname} --twin-id {twin_id} @@ -524,6 +534,10 @@ def load_digitaltwins_help(): - name: Remove a digital twin by Id. text: > az dt twin delete -n {instance_or_hostname} --twin-id {twin_id} + + - name: Remove a digital twin by Id using the etag. + text: > + az dt twin delete -n {instance_or_hostname} --twin-id {twin_id} --etag {etag} """ helps["dt twin relationship"] = """ @@ -542,6 +556,11 @@ def load_digitaltwins_help(): az dt twin relationship create -n {instance_or_hostname} --relationship-id {relationship_id} --relationship contains --twin-id {source_twin_id} --target {target_twin_id} + - name: Create a relationship between two digital twins with if-none-match tag + text: > + az dt twin relationship create -n {instance_or_hostname} --relationship-id {relationship_id} --relationship contains + --twin-id {source_twin_id} --target {target_twin_id} --if-none-match + - name: Create a relationship with initialized properties between two digital twins. text: > az dt twin relationship create -n {instance_or_hostname} --relationship-id {relationship_id} --relationship contains @@ -593,6 +612,11 @@ def load_digitaltwins_help(): az dt twin relationship update -n {instance_or_hostname} --twin-id {twin_id} --relationship-id {relationship_id} --relationship contains --json-patch '{"op":"replace", "path":"/Temperature", "value": 20.5}' + - name: Update a digital twin relationship via JSON patch specification and using etag. + text: > + az dt twin relationship update -n {instance_or_hostname} --twin-id {twin_id} --relationship-id {relationship_id} + --relationship contains --json-patch '{"op":"replace", "path":"/Temperature", "value": 20.5}' --etag {etag} + - name: Update a digital twin relationship via JSON patch specification. text: > az dt twin relationship update -n {instance_or_hostname} --twin-id {twin_id} --relationship-id {relationship_id} @@ -615,6 +639,10 @@ def load_digitaltwins_help(): - name: Delete a digital twin relationship. text: > az dt twin relationship delete -n {instance_or_hostname} --twin-id {twin_id} --relationship-id {relationship_id} + + - name: Delete a digital twin relationship using the etag. + text: > + az dt twin relationship delete -n {instance_or_hostname} --twin-id {twin_id} --relationship-id {relationship_id} --etag {etag} """ helps["dt twin telemetry"] = """ diff --git a/azext_iot/digitaltwins/params.py b/azext_iot/digitaltwins/params.py index 96b695655..5214425b5 100644 --- a/azext_iot/digitaltwins/params.py +++ b/azext_iot/digitaltwins/params.py @@ -279,7 +279,8 @@ def load_digitaltwins_arguments(self, _): "Operations are limited to add, replace and remove. Provide file path or inline JSON.", ) context.argument( - "etag", options_list=["--etag", "-e"], help="Entity tag value." + "etag", options_list=["--etag", "-e"], help="Entity tag value. The command will succeed if " + "the etag matches the current etag for the resource." ) context.argument( "component_path", diff --git a/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py b/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py index 4484aac78..1c884fee4 100644 --- a/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py +++ b/azext_iot/tests/digitaltwins/test_dt_twin_lifecycle_int.py @@ -30,6 +30,7 @@ def test_dt_twin(self): room_dtmi = "dtmi:com:example:Room;1" room_twin_id = "myroom" thermostat_component_id = "Thermostat" + etag = 'AAAA==' create_output = self.cmd( "dt create -n {} -g {} -l {}".format(instance_name, self.rg, self.region) @@ -141,6 +142,17 @@ def test_dt_twin(self): expect_failure=True ) + # delete command should fail if etag is different + self.cmd( + "dt twin delete -n {} -g {} --twin-id {} --etag '{}'".format( + instance_name, + self.rg, + room_twin_id, + etag + ), + expect_failure=True + ) + self.cmd( "dt twin delete -n {} -g {} --twin-id {}".format( instance_name, @@ -241,6 +253,30 @@ def test_dt_twin(self): == json.loads(self.kwargs["temperatureJsonPatch"])["value"] ) + self.cmd( + "dt twin update -n {} --twin-id {} --json-patch '{}' --etag '{}'".format( + instance_name, + room_twin_id, + "{temperatureJsonPatch}", + etag + ), + expect_failure=True + ) + + update_twin_result = self.cmd( + "dt twin update -n {} --twin-id {} --json-patch '{}' --etag '{}'".format( + instance_name, + room_twin_id, + "{temperatureJsonPatch}", + update_twin_result["$etag"] + ) + ).get_output_in_json() + + assert ( + update_twin_result["Temperature"] + == json.loads(self.kwargs["temperatureJsonPatch"])["value"] + ) + twin_query_result = self.cmd( "dt twin query -n {} -g {} -q 'select * from digitaltwins'".format( instance_name, self.rg @@ -257,6 +293,18 @@ def test_dt_twin(self): {"op": "replace", "path": "/ownershipUser", "value": "meme"} ) + twin_relationship_create_result = self.cmd( + "dt twin relationship create -n {} -g {} --relationship-id {} --relationship {} --twin-id {} " + "--target-twin-id {}".format( + instance_name, + self.rg, + relationship_id, + relationship, + floor_twin_id, + room_twin_id, + ) + ).get_output_in_json() + twin_relationship_create_result = self.cmd( "dt twin relationship create -n {} -g {} --relationship-id {} --relationship {} --twin-id {} " "--target-twin-id {} --properties '{}'".format( @@ -279,6 +327,21 @@ def test_dt_twin(self): properties=self.kwargs["relationshipJson"], ) + # new twin cannot be created with same twin_id if if-none-match provided + twin_relationship_create_result = self.cmd( + "dt twin relationship create -n {} -g {} --relationship-id {} --relationship {} --twin-id {} " + "--target-twin-id {} --if-none-match --properties '{}'".format( + instance_name, + self.rg, + relationship_id, + relationship, + floor_twin_id, + room_twin_id, + "{relationshipJson}", + ), + expect_failure=True + ) + twin_relationship_show_result = self.cmd( "dt twin relationship show -n {} -g {} --twin-id {} --relationship-id {}".format( instance_name, @@ -313,6 +376,37 @@ def test_dt_twin(self): == json.loads(self.kwargs["relationshipJsonPatch"])["value"] ) + # Fail to update if the etag if different + self.cmd( + "dt twin relationship update -n {} -g {} --relationship-id {} --twin-id {} " + "--json-patch '{}' --etag '{}'".format( + instance_name, + self.rg, + relationship_id, + floor_twin_id, + "{relationshipJsonPatch}", + etag + ), + expect_failure=True + ) + + twin_edge_update_result = self.cmd( + "dt twin relationship update -n {} -g {} --relationship-id {} --twin-id {} " + "--json-patch '{}' --etag '{}'".format( + instance_name, + self.rg, + relationship_id, + floor_twin_id, + "{relationshipJsonPatch}", + twin_edge_update_result["$etag"] + ) + ).get_output_in_json() + + assert ( + twin_edge_update_result["ownershipUser"] + == json.loads(self.kwargs["relationshipJsonPatch"])["value"] + ) + twin_relationship_list_result = self.cmd( "dt twin relationship list -n {} --twin-id {}".format( instance_name, @@ -354,6 +448,16 @@ def test_dt_twin(self): ).get_output_in_json() assert len(twin_relationship_list_result) == 1 + self.cmd( + "dt twin relationship delete -n {} --twin-id {} -r {} --etag '{}'".format( + instance_name, + floor_twin_id, + relationship_id, + etag + ), + expect_failure=True + ) + # No output from API for delete edge self.cmd( "dt twin relationship delete -n {} --twin-id {} -r {}".format(