-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add attribute name use checks to validation layer #8612
base: master
Are you sure you want to change the base?
Add attribute name use checks to validation layer #8612
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8612 +/- ##
==========================================
+ Coverage 92.62% 92.73% +0.10%
==========================================
Files 1229 1246 +17
Lines 106421 107985 +1564
==========================================
+ Hits 98576 100137 +1561
- Misses 7845 7848 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
moto/dynamodb/responses.py
Outdated
@@ -1142,6 +1258,25 @@ def transact_write_items(self) -> str: | |||
item_attrs = item["Put"]["Item"] | |||
table = self.dynamodb_backend.get_table(item["Put"]["TableName"]) | |||
validate_put_has_empty_keys(item_attrs, table) | |||
|
|||
condition_expression = item["Put"].get("ConditionExpression") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part seems to be the same for every action. Can we extract that somehow, to keep it DRY?
I don't want to prescribe exactly how you should do it, but I was thinking something along the lines of this:
for action in ["Put", ..]:
if details := item.get(action):
condition_expression = details.get("ConditionExpression")
...
Hi @BlizzardOf! You can use the environment variable All tests marked with To run all tests in a single file/folder against AWS:
Note the Or if you want to validate a single test:
Note that this particular test (
|
Hm, it's bizarre, I can test against AWS when I run each test individually, but when I run a bunch of them at once, they all fail with Whatever the issue is I can replicate the issue you mention, I should have updates up in a day or so. |
Alright, I think it's ready |
This addresses issue #8474
In the responses file, I added validation checks that every attribute name is used in the various expressions that each request takes. Without major changes, these validations have to be done at the responses level instead of the backend level, because some response functions call the same backend functions, and which expressions need to be included in the check varies between request types.
I had to adapt a couple of the parsers to track which attribute names they used during parsing.
I skipped batch_write_items because boto doesn't support any expressions in it, for some reason. I also skipped transact_get_items because it looks like moto doesn't support expressions for that call, even though moto does.
I used the aws.verified tag to get the tests working, but I actually haven't verified it against AWS. I was wondering if you had some tips for setting up a proper environment? I have no idea where to get started even.