From 54355705ed3dffc832aa1f3c756d7dd286a38567 Mon Sep 17 00:00:00 2001
From: ptrucks <75756293+ptrucks@users.noreply.github.com>
Date: Wed, 28 Apr 2021 13:10:37 -0700
Subject: [PATCH] Fix update generation with additionalIdentifiers (#732)

* Disable unhelpful pylint warning

pylint insists that `with` should be used on all disposable resources
but in this case the constructor call is in a helper method and the
consuming method does already use `with`. Given that this is a private
method it should be fine to ignore the warning in all cases.

* Fix update generation with additionalIdentifiers

If the `additionalIdentifiers` property is present in a resource schema
the `generate_update_example` method in `resource_client` will not work,
because it checks for unique keys from the create_model and passes
`self._additional_identifiers_paths` to a method that expects a list of
tuples of string, whereas it receives a list of sets of tuples of
string and will fail with

```
AttributeError: 'tuple' object has no attribute 'split'
```

Instead, check if the key is in any of the additional identifiers.

Related issue
-------------

https://github.com/aws-cloudformation/cloudformation-cli/issues/731
---
 src/rpdk/core/contract/resource_client.py |  9 ++++++++-
 src/rpdk/core/project.py                  |  2 ++
 tests/contract/test_resource_client.py    | 21 +++++++++++++++++----
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/rpdk/core/contract/resource_client.py b/src/rpdk/core/contract/resource_client.py
index 51b1182a..f181016d 100644
--- a/src/rpdk/core/contract/resource_client.py
+++ b/src/rpdk/core/contract/resource_client.py
@@ -290,7 +290,14 @@ def get_unique_keys_for_model(self, create_model):
             k: v
             for k, v in create_model.items()
             if self.is_property_in_path(k, self.primary_identifier_paths)
-            or self.is_property_in_path(k, self._additional_identifiers_paths)
+            or any(
+                map(
+                    lambda additional_identifier_paths, key=k: self.is_property_in_path(
+                        key, additional_identifier_paths
+                    ),
+                    self._additional_identifiers_paths,
+                )
+            )
         }
 
     @staticmethod
diff --git a/src/rpdk/core/project.py b/src/rpdk/core/project.py
index 62626651..ab59bc83 100644
--- a/src/rpdk/core/project.py
+++ b/src/rpdk/core/project.py
@@ -514,6 +514,8 @@ def _create_context_manager(self, dry_run):
         # if it's a dry run, keep the file; otherwise can delete after upload
         if dry_run:
             return self._get_zip_file_path().open("wb")
+
+        # pylint: disable=consider-using-with
         return TemporaryFile("w+b")
 
     def _get_zip_file_path(self):
diff --git a/tests/contract/test_resource_client.py b/tests/contract/test_resource_client.py
index 499b6435..da55c0ab 100644
--- a/tests/contract/test_resource_client.py
+++ b/tests/contract/test_resource_client.py
@@ -80,6 +80,19 @@
     "primaryIdentifier": ["/properties/c", "/properties/d"],
 }
 
+SCHEMA_WITH_ADDITIONAL_IDENTIFIERS = {
+    "properties": {
+        "a": {"type": "number"},
+        "b": {"type": "number"},
+        "c": {"type": "number"},
+        "d": {"type": "number"},
+    },
+    "readOnlyProperties": ["/properties/b"],
+    "createOnlyProperties": ["/properties/c"],
+    "primaryIdentifier": ["/properties/c"],
+    "additionalIdentifiers": [["/properties/b"]],
+}
+
 
 @pytest.fixture
 def resource_client():
@@ -157,8 +170,8 @@ def resource_client_inputs():
     return client
 
 
-@pytest.fixture
-def resource_client_inputs_schema():
+@pytest.fixture(params=[SCHEMA_, SCHEMA_WITH_ADDITIONAL_IDENTIFIERS])
+def resource_client_inputs_schema(request):
     endpoint = "https://"
     patch_sesh = patch(
         "rpdk.core.contract.resource_client.create_sdk_session", autospec=True
@@ -181,7 +194,7 @@ def resource_client_inputs_schema():
                 DEFAULT_FUNCTION,
                 endpoint,
                 DEFAULT_REGION,
-                SCHEMA_,
+                request.param,
                 EMPTY_OVERRIDE,
                 {
                     "CREATE": {"a": 111, "c": 2, "d": 3},
@@ -195,7 +208,7 @@ def resource_client_inputs_schema():
     mock_account.assert_called_once_with(mock_sesh, {})
 
     assert client._function_name == DEFAULT_FUNCTION
-    assert client._schema == SCHEMA_
+    assert client._schema == request.param
     assert client._overrides == EMPTY_OVERRIDE
     assert client.account == ACCOUNT