Delete & Read Handler should be invoked using identifiers (#537)

* Delete & Read Handler should be invoked using identifiers

* Allow schema merge to happen iteratively for a path

* Add suggested changes

* update the handler

Author: anshikg

src/rpdk/core/cli.py

@@ -104,6 +104,7 @@ def no_command(args):
         # some cases where it makes sense for the commands to raise SystemExit.)
         log.debug("Caught exit recommendation", exc_info=e)
         log.critical(str(e))
+        # pylint: disable=W0707
         raise SystemExit(1)
     except DownstreamError as e:
         # For these operations, we don't want to swallow the exception
@@ -124,6 +125,7 @@ def no_command(args):
             "https://github.com/aws-cloudformation/aws-cloudformation-rpdk/issues",
             file=sys.stderr,
         )
+        # pylint: disable=W0707
         raise SystemExit(2)
     except Exception:  # pylint: disable=broad-except
         print("=== Unhandled exception ===", file=sys.stderr)
@@ -142,4 +144,5 @@ def no_command(args):
             import traceback  # pylint: disable=import-outside-toplevel
 
             traceback.print_exc()
+        # pylint: disable=W0707
         raise SystemExit(EXIT_UNHANDLED_EXCEPTION)

src/rpdk/core/contract/resource_client.py

@@ -98,6 +98,22 @@ def override_properties(document, overrides):
     return document
 
 
+def create_model_with_properties_in_path(src_model, paths):
+    """Creates a model with values preset in the paths.
+
+    This assumes properties will always have an object (dict) as a parent.
+    The function returns the created model.
+    """
+    model = {}
+    try:
+        for path in paths:
+            pruned_path = path[-1]
+            model[pruned_path] = src_model[pruned_path]
+    except LookupError:
+        pass
+    return model
+
+
 class ResourceClient:  # pylint: disable=too-many-instance-attributes
     def __init__(
         self,

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

@@ -1,3 +1,4 @@
+import logging
 from functools import wraps
 from inspect import Parameter, signature
 from unittest.test.testmock.support import is_instance
@@ -6,6 +7,8 @@
 
 from rpdk.core.contract.interface import HandlerErrorCode
 
+LOG = logging.getLogger(__name__)
+
 
 def _rebind(decorator, func, *args, **kwargs):
     """Helper function to construct decorated arguments
@@ -82,6 +85,8 @@ def response_does_not_contain_write_only_properties(resource_client, response):
 def response_contains_resource_model_equal_current_model(
     response, current_resource_model
 ):
+    LOG.debug("This is response[resourceModel] %s", response["resourceModel"])
+    LOG.debug("This is current_resource_model %s", current_resource_model)
     assert response["resourceModel"] == current_resource_model
 
 

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

@@ -2,6 +2,7 @@
 
 from rpdk.core.contract.interface import Action, HandlerErrorCode, OperationStatus
 from rpdk.core.contract.resource_client import (
+    create_model_with_properties_in_path,
     prune_properties_from_model,
     prune_properties_if_not_exist_in_path,
 )
@@ -48,16 +49,22 @@ def test_create_failure_if_repeat_writeable_id(resource_client, current_resource
 @response_does_not_contain_write_only_properties
 @response_contains_resource_model_equal_current_model
 def test_read_success(resource_client, current_resource_model):
+    primay_identifier_only_model = create_model_with_properties_in_path(
+        current_resource_model.copy(), resource_client.primary_identifier_paths,
+    )
     _status, response, _error_code = resource_client.call_and_assert(
-        Action.READ, OperationStatus.SUCCESS, current_resource_model
+        Action.READ, OperationStatus.SUCCESS, primay_identifier_only_model
     )
     return response
 
 
 @failed_event(error_code=HandlerErrorCode.NotFound)
 def test_read_failure_not_found(resource_client, current_resource_model):
+    primay_identifier_only_model = create_model_with_properties_in_path(
+        current_resource_model, resource_client.primary_identifier_paths,
+    )
     _status, _response, error_code = resource_client.call_and_assert(
-        Action.READ, OperationStatus.FAILED, current_resource_model
+        Action.READ, OperationStatus.FAILED, primay_identifier_only_model
     )
     return error_code
 
@@ -116,30 +123,36 @@ def test_update_failure_not_found(resource_client, current_resource_model):
 
 
 def test_delete_success(resource_client, current_resource_model):
+    primay_identifier_only_model = create_model_with_properties_in_path(
+        current_resource_model, resource_client.primary_identifier_paths,
+    )
     _status, response, _error_code = resource_client.call_and_assert(
-        Action.DELETE, OperationStatus.SUCCESS, current_resource_model
+        Action.DELETE, OperationStatus.SUCCESS, primay_identifier_only_model
     )
     return response
 
 
 @failed_event(error_code=HandlerErrorCode.NotFound)
 def test_delete_failure_not_found(resource_client, current_resource_model):
+    primay_identifier_only_model = create_model_with_properties_in_path(
+        current_resource_model, resource_client.primary_identifier_paths,
+    )
     _status, _response, error_code = resource_client.call_and_assert(
-        Action.DELETE, OperationStatus.FAILED, current_resource_model
+        Action.DELETE, OperationStatus.FAILED, primay_identifier_only_model
     )
     return error_code
 
 
 def test_input_equals_output(resource_client, input_model, output_model):
-    input_model = prune_properties_from_model(
-        input_model, resource_client.write_only_paths
+    pruned_input_model = prune_properties_from_model(
+        input_model.copy(), resource_client.write_only_paths
     )
 
-    output_model = prune_properties_from_model(
-        output_model, resource_client.read_only_paths
+    pruned_output_model = prune_properties_if_not_exist_in_path(
+        output_model.copy(), pruned_input_model, resource_client.read_only_paths
     )
-    output_model = prune_properties_if_not_exist_in_path(
-        output_model, input_model, resource_client.create_only_paths
+    pruned_output_model = prune_properties_if_not_exist_in_path(
+        pruned_output_model, pruned_input_model, resource_client.create_only_paths
     )
 
-    assert input_model == output_model
+    assert pruned_input_model == pruned_output_model

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

@@ -7,6 +7,7 @@
 # WARNING: contract tests should use fully qualified imports to avoid issues
 # when being loaded by pytest
 from rpdk.core.contract.interface import Action, HandlerErrorCode, OperationStatus
+from rpdk.core.contract.resource_client import create_model_with_properties_in_path
 from rpdk.core.contract.suite.contract_asserts import (
     failed_event,
     skip_not_writable_identifier,
@@ -34,7 +35,12 @@ def created_resource(resource_client):
         test_input_equals_output(resource_client, input_model, model)
         yield model, request
     finally:
-        resource_client.call_and_assert(Action.DELETE, OperationStatus.SUCCESS, model)
+        primay_identifier_only_model = create_model_with_properties_in_path(
+            model, resource_client.primary_identifier_paths
+        )
+        resource_client.call_and_assert(
+            Action.DELETE, OperationStatus.SUCCESS, primay_identifier_only_model
+        )
 
 
 @pytest.mark.create

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

@@ -7,7 +7,10 @@
 # WARNING: contract tests should use fully qualified imports to avoid issues
 # when being loaded by pytest
 from rpdk.core.contract.interface import Action, HandlerErrorCode, OperationStatus
-from rpdk.core.contract.resource_client import prune_properties_from_model
+from rpdk.core.contract.resource_client import (
+    create_model_with_properties_in_path,
+    prune_properties_from_model,
+)
 from rpdk.core.contract.suite.handler_commons import (
     test_create_success,
     test_delete_failure_not_found,
@@ -25,20 +28,23 @@
 
 @pytest.fixture(scope="module")
 def deleted_resource(resource_client):
-    request = input_model = model = resource_client.generate_create_example()
+    request = input_model = pruned_model = resource_client.generate_create_example()
     try:
         _status, response, _error = resource_client.call_and_assert(
             Action.CREATE, OperationStatus.SUCCESS, request
         )
         model = response["resourceModel"]
         test_input_equals_output(resource_client, input_model, model)
+        primay_identifier_only_model = create_model_with_properties_in_path(
+            model, resource_client.primary_identifier_paths,
+        )
         _status, response, _error = resource_client.call_and_assert(
-            Action.DELETE, OperationStatus.SUCCESS, model
+            Action.DELETE, OperationStatus.SUCCESS, primay_identifier_only_model
         )
         assert "resourceModel" not in response
         yield model, request
     finally:
-        status, response = resource_client.call(Action.DELETE, model)
+        status, response = resource_client.call(Action.DELETE, pruned_model)
 
         # a failed status is allowed if the error code is NotFound
         if status == OperationStatus.FAILED:

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

@@ -6,6 +6,7 @@
 # WARNING: contract tests should use fully qualified imports to avoid issues
 # when being loaded by pytest
 from rpdk.core.contract.interface import Action, OperationStatus
+from rpdk.core.contract.resource_client import create_model_with_properties_in_path
 from rpdk.core.contract.suite.handler_commons import (
     test_input_equals_output,
     test_model_in_list,
@@ -34,7 +35,12 @@ def updated_resource(resource_client):
 
         yield create_request, created_model, update_request, updated_model
     finally:
-        resource_client.call_and_assert(Action.DELETE, OperationStatus.SUCCESS, model)
+        primay_identifier_only_model = create_model_with_properties_in_path(
+            model, resource_client.primary_identifier_paths
+        )
+        resource_client.call_and_assert(
+            Action.DELETE, OperationStatus.SUCCESS, primay_identifier_only_model
+        )
 
 
 @pytest.mark.update

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

@@ -6,6 +6,7 @@
 # WARNING: contract tests should use fully qualified imports to avoid issues
 # when being loaded by pytest
 from rpdk.core.contract.interface import Action, HandlerErrorCode, OperationStatus
+from rpdk.core.contract.resource_client import create_model_with_properties_in_path
 from rpdk.core.contract.suite.contract_asserts import failed_event
 
 
@@ -28,8 +29,11 @@ def contract_update_create_only_property(resource_client):
             )
             assert response["message"]
         finally:
+            primay_identifier_only_model = create_model_with_properties_in_path(
+                created_model, resource_client.primary_identifier_paths,
+            )
             resource_client.call_and_assert(
-                Action.DELETE, OperationStatus.SUCCESS, created_model
+                Action.DELETE, OperationStatus.SUCCESS, primay_identifier_only_model
             )
     else:
         pytest.skip("No createOnly Properties. Skipping test.")

src/rpdk/core/init.py

@@ -68,6 +68,7 @@ def __call__(self, value):
         try:
             choice = int(value)
         except ValueError:
+            # pylint: disable=W0707
             raise WizardValidationError("Please enter an integer")
         choice -= 1
         if choice < 0 or choice >= self.max:
@@ -159,6 +160,7 @@ def wrapper(args):
             function(args)
         except (KeyboardInterrupt, WizardAbortError):
             print("\naborted")
+            # pylint: disable=W0707
             raise SystemExit(1)
 
     return wrapper

src/rpdk/core/jsonutils/flattener.py

@@ -88,6 +88,7 @@ def _flatten_ref_type(self, ref_path):
             try:
                 ref_parts = fragment_decode(ref_path)
             except ValueError as e:
+                # pylint: disable=W0707
                 raise FlatteningError(
                     "Invalid ref at path '{}': {}".format(ref_path, str(e))
                 )
@@ -190,4 +191,5 @@ def _find_subschema_by_ref(self, ref_path):
         try:
             return traverse(self._full_schema, ref_path)
         except (LookupError, ValueError):
+            # pylint: disable=W0707
             raise FlatteningError("Invalid ref: {}".format(ref_path))