Fix writable identifier check for composite key primary identifier (#598

)

* Update has_writable_identifiers:
instead of checking if both primaryIdentifers and additionalIdentifiers are writable, we're only checking if any primaryIdentifiers are createOnly

* Updated the function name appropriately

src/rpdk/core/contract/resource_client.py

@@ -173,15 +173,10 @@ def _update_schema(self, schema):
             for identifier in additional_identifiers
         ]
 
-    def has_writable_identifier(self):
-        for path in self.primary_identifier_paths:
-            if path not in self.read_only_paths:
-                return True
-        for identifier_paths in self._additional_identifiers_paths:
-            for path in identifier_paths:
-                if path not in self.read_only_paths:
-                    return True
-        return False
+    def has_only_writable_identifiers(self):
+        return all(
+            path in self.create_only_paths for path in self.primary_identifier_paths
+        )
 
     def assert_write_only_property_does_not_exist(self, resource_model):
         if self.write_only_paths:

src/rpdk/core/contract/suite/contract_asserts.py

@@ -121,7 +121,7 @@ def response_contains_unchanged_primary_identifier(
 
 @decorate(after=False)
 def skip_not_writable_identifier(resource_client):
-    if not resource_client.has_writable_identifier():
+    if not resource_client.has_only_writable_identifiers():
         pytest.skip("No writable identifiers. Skipping test.")
 
 

src/rpdk/core/contract/suite/handler_delete.py

@@ -90,7 +90,7 @@ def contract_delete_delete(resource_client, deleted_resource):
 @pytest.mark.create
 @pytest.mark.delete
 def contract_delete_create(resource_client, deleted_resource):
-    if resource_client.has_writable_identifier():
+    if resource_client.has_only_writable_identifiers():
         deleted_model, request = deleted_resource
         response = test_create_success(resource_client, request)
         created_response = response.copy()

tests/contract/test_resource_client.py

@@ -449,24 +449,31 @@ def test_generate_update_example_create_override(resource_client):
     assert example == {"a": 5, "b": 2}
 
 
-def test_has_writable_identifier_primary_is_read_only(resource_client):
+def test_has_only_writable_identifiers_primary_is_read_only(resource_client):
     resource_client._update_schema(
         {
             "primaryIdentifier": ["/properties/foo"],
             "readOnlyProperties": ["/properties/foo"],
         }
     )
 
-    assert not resource_client.has_writable_identifier()
+    assert not resource_client.has_only_writable_identifiers()
 
 
-def test_has_writable_identifier_primary_is_writeable(resource_client):
-    resource_client._update_schema({"primaryIdentifier": ["/properties/foo"]})
+def test_has_only_writable_identifiers_primary_is_writable(resource_client):
+    resource_client._update_schema(
+        {
+            "primaryIdentifier": ["/properties/foo"],
+            "createOnlyProperties": ["/properties/foo"],
+        }
+    )
 
-    assert resource_client.has_writable_identifier()
+    assert resource_client.has_only_writable_identifiers()
 
 
-def test_has_writable_identifier_primary_and_additional_are_read_only(resource_client):
+def test_has_only_writable_identifiers_primary_and_additional_are_read_only(
+    resource_client,
+):
     resource_client._update_schema(
         {
             "primaryIdentifier": ["/properties/foo"],
@@ -475,10 +482,10 @@ def test_has_writable_identifier_primary_and_additional_are_read_only(resource_c
         }
     )
 
-    assert not resource_client.has_writable_identifier()
+    assert not resource_client.has_only_writable_identifiers()
 
 
-def test_has_writable_identifier_additional_is_writeable(resource_client):
+def test_has_only_writable_identifiers_additional_is_writable(resource_client):
     resource_client._update_schema(
         {
             "primaryIdentifier": ["/properties/foo"],
@@ -487,10 +494,10 @@ def test_has_writable_identifier_additional_is_writeable(resource_client):
         }
     )
 
-    assert resource_client.has_writable_identifier()
+    assert not resource_client.has_only_writable_identifiers()
 
 
-def test_has_writable_identifier_compound_is_writeable(resource_client):
+def test_has_only_writable_identifiers_compound_is_writable(resource_client):
     resource_client._update_schema(
         {
             "primaryIdentifier": ["/properties/foo"],
@@ -499,7 +506,47 @@ def test_has_writable_identifier_compound_is_writeable(resource_client):
         }
     )
 
-    assert resource_client.has_writable_identifier()
+    assert not resource_client.has_only_writable_identifiers()
+
+
+def test_has_only_writable_identifiers_composite_primary_are_read_only(
+    resource_client,
+):
+    resource_client._update_schema(
+        {
+            "primaryIdentifier": ["/properties/foo", "/properties/bar"],
+            "readOnlyProperties": ["/properties/foo", "/properties/bar"],
+        }
+    )
+
+    assert not resource_client.has_only_writable_identifiers()
+
+
+def test_has_only_writable_identifiers_composite_primary_is_read_only(
+    resource_client,
+):
+    resource_client._update_schema(
+        {
+            "primaryIdentifier": ["/properties/foo", "/properties/bar"],
+            "readOnlyProperties": ["/properties/foo"],
+            "createOnlyProperties": ["/properties/bar"],
+        }
+    )
+
+    assert not resource_client.has_only_writable_identifiers()
+
+
+def test_has_only_writable_identifiers_composite_primary_are_writable(
+    resource_client,
+):
+    resource_client._update_schema(
+        {
+            "primaryIdentifier": ["/properties/foo", "/properties/bar"],
+            "createOnlyProperties": ["/properties/foo", "/properties/bar"],
+        }
+    )
+
+    assert resource_client.has_only_writable_identifiers()
 
 
 def test_make_payload(resource_client):