More public documentation/guidance on each contract test

[nina-ctrlv]

We provide a contract for handlers to follow in the docs, but it will be useful for providers to know what each individual test within the contract tests is trying to accomplish, what the inputs are for the test, and what the expected outcome is without having to read through the code of the test. Might also be useful to have more context on the test failures when we're reporting the results as well (ie why it's bad the test is failing, suggestions on how to fix it).

[advaj]

+1 - As someone who is migrating resources at scale. From personal experience, we have come up with some notes like - But if the tests have suggestions and what has failed specifically, that would be very useful.

1. Delete Handlers should not return a Model. So, when the Handler is returning a Success Progress Event, make sure that the Resource Model attribute is set to null. 
2. The Update, Read and Delete Handlers should contain a check for the existence of a Primary Identifier in the Model. If the Primary Identifier doesn’t exist, then throw a NotFound Error Code. 
3. If your Schema contains create only properties, make sure you include a check in the Update Handler for verifying if these properties are not being changed. 

Similarly, most contract tests check for something specific like this. Hence, some documentation about what each test is checking for and a potential resolution would be very useful. It would save us a ton of time as well. In some cases, it is taking us more time to fix contract tests than write the handler code.

[PatMyron]

Clearer contract test names themselves are needed. I fixed a ListHandler to pass a contract test without list in the name; seems unintuitive even if more documentation existed

[anshikg]

Re-opening this issue. It looks like we missed the documentation release here

[PatMyron]

3. If your Schema contains create only properties, make sure you include a check in the Update Handler for verifying if these properties are not being changed.

Why should every UpdateHandler handle this for every createOnlyProperty? The point of createOnlyProperties was so we didn't have to re-implement this

2. The Update, Read and Delete Handlers should contain a check for the existence of a Primary Identifier in the Model. If the Primary Identifier doesn’t exist, then throw a NotFound Error Code.

Again, if this always must be done, why rely on all these Handlers implementing this correctly anyway?

1. Delete Handlers should not return a Model. So, when the Handler is returning a Success Progress Event, make sure that the Resource Model attribute is set to null.

Again, why does every DeleteHandler need to do the same thing to the ResourceModel when returning a successful ProgressEvent?

---

All seem better handled levels above individual resource providers

[nina-ctrlv]

Why should every UpdateHandler have to handle this for every createOnlyProperty? I thought the point of createOnlyProperties was so we didn't have to worry about this

Completely agree. These contract tests should be removed this logic should be handled by cloudformation.

Wondering the same for this as well. If this always must be done, why rely on all of these Handlers implementing this correctly anyway?

In terms of checking whether the resource exists, I do think it should be handled by each handler. The error codes and exceptions thrown will be unique for whatever service it is. Providers will still need to make the translation so cfn understands. Cloudformation could use the read handler in order check before each action but the provider still needs to implement it correctly with the read handler.

Same for this as well. Why does every DeleteHandler need to do the exact same thing to the ResourceModel when returning a successful ProgressEvent?

I agree as well 👍

[PatMyron]

In terms of checking whether the resource exists, I do think it should be handled by each handler. The error codes and exceptions thrown will be unique for whatever service it is. Providers will still need to make the translation so cfn understands. Cloudformation could use the read handler in order check before each action but the provider still needs to implement it correctly with the read handler.

In general, I agree with this if a potentially valid primaryIdentifier is in the ResourceModel but the resource just does not exist. But if the identifier isn't even in the ResourceModel or doesn't even match the identifier constraints, I don't think we need individual Update, Read and Delete handlers to handle that. Update, Read and Delete Handlers should not be called without a potentially valid identifier and downstream service calls shouldn't be necessary to know a resource should not be found without a potentially valid identifier

[advaj]

Also, if Physical ID = Primary Identifier, then Updates and Deletes will always have an ID present when they are invoked? Why should this be checked at all?

[benbridts]

Update, Read and Delete Handlers should never be called without a potentially valid primaryIdentifier and downstream service calls shouldn't be necessary to know a resource should not be found without a potentially valid primaryIdentifier

I agree. However, it does make sense to phrase it as "If the Update, Read and Delete Handlers are invoked for a not-existing primaryIdentifier, they should throw a NotFound Error Code."

As opposed to:

• throwing another error
• trying to identify the resource based on other properties