Improving logging for Ctv1 (#996)

* Improving logging for Ctv1

Author: rajdnp

src/rpdk/core/contract/resource_client.py

@@ -281,12 +281,20 @@ def has_only_writable_identifiers(self):
         )
 
     def assert_write_only_property_does_not_exist(self, resource_model):
+
+        error_list = []
         if self.write_only_paths:
-            assert not any(
-                self.key_error_safe_traverse(resource_model, write_only_property)
-                for write_only_property in self.write_only_paths
-            ), "The model MUST NOT return properties defined as \
-                writeOnlyProperties in the resource schema"
+            for write_only_property in self.write_only_paths:
+                val = self.key_error_safe_traverse(resource_model, write_only_property)
+                if val:
+                    error_list.append(write_only_property[1])
+            assertion_error_message = (
+                "The model MUST NOT return properties defined as "
+                "writeOnlyProperties in the resource schema "
+                "\n Write only properties in resource model : %s  \n Output Resource Model : %s \n"
+                % (error_list, resource_model)
+            )
+            assert not any(error_list), assertion_error_message
 
     def get_metadata(self):
         try:
@@ -450,7 +458,8 @@ def compare_model(self, inputs, outputs, path=()):
             "All properties specified in the request MUST "
             "be present in the model returned, and they MUST"
             " match exactly, with the exception of properties"
-            " defined as writeOnlyProperties in the resource schema"
+            " defined as writeOnlyProperties in the resource schema \n Request Model : %s \n Returned Model : %s \n"
+            % (inputs, outputs)
         )
         try:
             if isinstance(inputs, dict):
@@ -476,6 +485,16 @@ def compare_model(self, inputs, outputs, path=()):
                             new_path,
                         )
                     else:
+                        if inputs[key] != outputs[key]:
+                            assertion_error_message = (
+                                "%s Value for property %s in Request Model(%s) and Response Model(%s) does not match"
+                                % (
+                                    assertion_error_message,
+                                    key,
+                                    inputs[key],
+                                    outputs[key],
+                                )
+                            )
                         assert inputs[key] == outputs[key], assertion_error_message
             else:
                 assert inputs == outputs, assertion_error_message
@@ -628,6 +647,22 @@ def is_primary_identifier_equal(
                      match the primaryIdentifier passed into the request"
             ) from e
 
+    @staticmethod
+    def get_primary_identifier(primary_identifier_path, model):
+        try:
+            pid_list = []
+            for primary_identifier in primary_identifier_path:
+                data = traverse(model, fragment_list(primary_identifier, "properties"))[
+                    0
+                ]
+                pid_list.append(data)
+            return pid_list
+        except KeyError as e:
+            raise AssertionError(
+                "The primaryIdentifier returned in every progress event must\
+                     match the primaryIdentifier passed into the request \n"
+            ) from e
+
     def _make_payload(
         self,
         action,

src/rpdk/core/contract/suite/resource/handler_commons.py

@@ -102,6 +102,33 @@ def test_model_in_list(resource_client, current_resource_model):
     )
 
 
+def error_test_model_in_list(resource_client, current_resource_model, message):
+    resource_models = get_resource_model_list(resource_client, current_resource_model)
+    assertion_error_message = message
+    for resource_model in resource_models:
+        resource_model_primary_identifier = resource_client.get_primary_identifier(
+            resource_client.primary_identifier_paths, resource_model
+        )
+        current_model_primary_identifier = resource_client.get_primary_identifier(
+            resource_client.primary_identifier_paths, current_resource_model
+        )
+        if resource_model_primary_identifier != current_model_primary_identifier:
+            assertion_error_message = (
+                "%s \n Resource Model primary identifier %s does not match with "
+                "Current Resource Model primary identifier %s \n Resource Model : %s"
+                " \n Currrent Model : %s "
+                % (
+                    message,
+                    resource_model_primary_identifier[0],
+                    current_model_primary_identifier[0],
+                    resource_model,
+                    current_resource_model,
+                )
+            )
+            return assertion_error_message
+    return assertion_error_message
+
+
 @response_contains_primary_identifier
 @response_contains_unchanged_primary_identifier
 @response_contains_resource_model_equal_updated_model

src/rpdk/core/contract/suite/resource/handler_create.py

@@ -13,6 +13,7 @@
     skip_not_writable_identifier,
 )
 from rpdk.core.contract.suite.resource.handler_commons import (
+    error_test_model_in_list,
     test_create_failure_if_repeat_writeable_id,
     test_create_success,
     test_delete_success,
@@ -75,7 +76,9 @@ def contract_create_read(created_resource, resource_client):
 @pytest.mark.read
 def contract_create_list(created_resource, resource_client):
     _input_model, created_model, _request = created_resource
-    assert test_model_in_list(resource_client, created_model)
+    assert test_model_in_list(resource_client, created_model), error_test_model_in_list(
+        resource_client, created_model, ""
+    )
     test_read_success(resource_client, created_model)
 
 

src/rpdk/core/contract/suite/resource/handler_update.py

@@ -11,6 +11,7 @@
     skip_not_tag_updatable,
 )
 from rpdk.core.contract.suite.resource.handler_commons import (
+    error_test_model_in_list,
     test_input_equals_output,
     test_model_in_list,
     test_read_success,
@@ -55,10 +56,26 @@ def contract_update_read(updated_resource, resource_client):
         updated_model,
         updated_input_model,
     ) = updated_resource
+    create_primary_identifiers = resource_client.get_primary_identifier(
+        resource_client.primary_identifier_paths, _created_model
+    )
+    update_primary_identifiers = resource_client.get_primary_identifier(
+        resource_client.primary_identifier_paths, updated_model
+    )
+    assertion_error_message = (
+        "The primaryIdentifier returned must match "
+        "the primaryIdentifier passed into the request "
+        "Create Model primary identifier %s does not match with Update Model primary identifier %s \n Create Model : %s \n Update Model : %s "
+        % (
+            create_primary_identifiers,
+            update_primary_identifiers,
+            _created_model,
+            updated_model,
+        )
+    )
     assert resource_client.is_primary_identifier_equal(
         resource_client.primary_identifier_paths, _created_model, updated_model
-    ), "The primaryIdentifier returned must match\
-         the primaryIdentifier passed into the request"
+    ), assertion_error_message
     read_response = test_read_success(resource_client, updated_model)
     test_input_equals_output(
         resource_client, updated_input_model, read_response["resourceModel"]
@@ -81,9 +98,11 @@ def contract_update_list(updated_resource, resource_client):
         resource_client.primary_identifier_paths, _created_model, updated_model
     ), "The primaryIdentifier returned must match\
          the primaryIdentifier passed into the request"
-    assert test_model_in_list(
-        resource_client, updated_model
-    ), "A list handler MUST always return an updated model"
+    assert test_model_in_list(resource_client, updated_model), error_test_model_in_list(
+        resource_client,
+        updated_model,
+        "A list handler MUST always return an updated model",
+    )
 
 
 @pytest.mark.update

tests/contract/test_resource_client.py

@@ -7,6 +7,7 @@
 
 import pytest
 
+import rpdk.core.contract.resource_client as rclient
 from rpdk.core.boto_helpers import LOWER_CAMEL_CRED_KEYS
 from rpdk.core.contract.interface import Action, HandlerErrorCode, OperationStatus
 from rpdk.core.contract.resource_client import (
@@ -17,6 +18,7 @@
     prune_properties_if_not_exist_in_path,
     prune_properties_which_dont_exist_in_path,
 )
+from rpdk.core.contract.suite.resource.handler_commons import error_test_model_in_list
 from rpdk.core.exceptions import InvalidProjectError
 from rpdk.core.test import (
     DEFAULT_ENDPOINT,
@@ -149,6 +151,8 @@
 
 EMPTY_SCHEMA = {"handlers": {"create": [], "delete": [], "read": []}}
 
+RESOURCE_MODEL_LIST = [{"Id": "abc123", "b": "2"}]
+
 
 @pytest.fixture
 def resource_client():
@@ -390,6 +394,42 @@ def resource_client_inputs_property_transform():
     return client
 
 
+def test_error_test_model_in_list(resource_client):
+    patch_resource_model_list = patch(
+        "rpdk.core.contract.suite.resource.handler_commons.get_resource_model_list",
+        autospec=True,
+        return_value=RESOURCE_MODEL_LIST,
+    )
+    resource_client.primary_identifier_paths = {("properties", "Id")}
+    current_resource_model = {"Id": "xyz456", "b": "2"}
+    with patch_resource_model_list:
+        assertion_error_message = error_test_model_in_list(
+            resource_client, current_resource_model, ""
+        )
+        assert (
+            "abc123 does not match with Current Resource Model primary identifier xyz456"
+            in assertion_error_message
+        )
+
+
+def test_get_primary_identifier_success():
+    primary_identifier_path = {("properties", "a")}
+    model = {"a": 1, "b": 3, "c": 4}
+    plist = rclient.ResourceClient.get_primary_identifier(
+        primary_identifier_path, model
+    )
+    assert plist[0] == 1
+
+
+def test_get_primary_identifier_fail():
+    primary_identifier_path = {("properties", "a")}
+    model = {"b": 3, "c": 4}
+    try:
+        rclient.ResourceClient.get_primary_identifier(primary_identifier_path, model)
+    except AssertionError:
+        logging.debug("This test expects Assertion Exception to be thrown")
+
+
 def test_prune_properties():
     document = {
         "foo": "bar",