added some pre-actionable checks for update and read handler to align with handler contract #294
Conversation
… with handler contract
ammokhov
added
code quality
codegen
| @@ -52,6 +52,7 @@ static ResourceModel translateFromReadResponse(final AwsResponse awsResponse) { | |||
| // e.g. https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-logs/blob/2077c92299aeb9a68ae8f4418b5e932b12a8b186/aws-logs-loggroup/src/main/java/com/aws/logs/loggroup/Translator.java#L58-L73 | |||
| return ResourceModel.builder() | |||
| //.someProperty(response.property()) | |||
| // IMPORTANT: exclude writeOnlyProperties | |||
There was a problem hiding this comment.
isn't this done by lambda wrapper?
There was a problem hiding this comment.
Yes it should be. At least in this language plugin.
There was a problem hiding this comment.
could you just remove this line about write only properties. This might cause some confusion.
|
|
||
| // STEP 1 [check if resource already exists] | ||
| // for more information -> https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/resource-type-test-contract.html | ||
| // if target API does not support 'ResourceNotFoundException' then following check is required |
There was a problem hiding this comment.
it seems we are checking for not found at multiple places. May be consolidate this to be in on place.
There was a problem hiding this comment.
we might be having same callGraph value cached in case you cross handler interaction. also I think it's already quite wordy if we refer in multiple places it will be confusing
PatMyron
changed the title
| .then(progress -> | ||
| // STEP 1.0 [initialize a proxy context] | ||
| // If your service API does not return ResourceNotFoundException on update requests against some identifier (e.g; resource Name) | ||
| // and instead returns a 200 even though a resource does not exist, you must first check if the resource exists here | ||
| // NOTE: If your service API throws 'ResourceNotFoundException' for update requests this method is not necessary |
There was a problem hiding this comment.
Even if the service API throws ResourceNotFoundException, the contract tests as currently implemented expect ResourceNotFoundException to be thrown even when calling the handlers with obviously invalid inputs which likely will hit model validation before checking if the resource exists or not, even for services that do throw ResourceNotFoundException
also discussed in aws-cloudformation/cloudformation-cli#527
Codegen enhancements to provide to a developer additional tools for contract test passing:
An update handler MUST return FAILED with a NotFound error code if the resource did not exist prior to the update request.
When the delete handler returns SUCCESS, the ProgressEvent object MUST NOT contain a model.
A read handler MUST return FAILED with a NotFound error code if the resource does not exist.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.