From 65028a59af10f60874f60dc648b03b5dc6df1432 Mon Sep 17 00:00:00 2001 From: vilit1 <73560279+vilit1@users.noreply.github.com> Date: Thu, 25 Feb 2021 13:02:26 -0800 Subject: [PATCH] ADT refactor --replace to --if-none-match (#315) * Use if_none_match instead of replace * pylint error fix * update help messaages * update help and params, add etags to it --- azext_iot/digitaltwins/_help.py | 28 +++++ azext_iot/digitaltwins/commands_twins.py | 8 +- azext_iot/digitaltwins/params.py | 9 +- azext_iot/digitaltwins/providers/twin.py | 8 +- azext_iot/operations/hub.py | 2 +- .../test_dt_twin_lifecycle_int.py | 110 +++++++++++++++++- .../tests/digitaltwins/test_dt_twins_unit.py | 16 +-- 7 files changed, 157 insertions(+), 24 deletions(-) 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/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..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", @@ -287,9 +288,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 if an existing twin with the same id 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/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) 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..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) @@ -112,7 +113,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 +130,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, @@ -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( 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: