Handle unprocessed items in batch write responses

[woodyza]

Addresses #92, #91

Changes the encrypt_batch_write_item utility method to check for UnprocessedItems in the response, and replace them with the original plaintext items.

Couple talking points:

• The encryption process is currently overwriting the plaintext item value, and in order to enable reverting it back I needed to deep copy the RequestItems.
• Handling the possible lack of schema information when searching out the correct plaintext item meant having a fallback that requires all the attributes not marked for encryption to match; a brute-force way of making sure the key attributes match since they aren't encrypted. Not sure if there's a more elegant way.
• I may have put my tests in the wrong place :)
• My tests are low-level implementation tests, should there be a higher-level test that triggers throughput exceeded for e.g. to cover regressions on the higher-level requirement?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[mattsb42-aws]

Hi @woodyza, thanks for this! I'm taking a look at the code today.

should there be a higher-level test that triggers throughput exceeded for e.g. to cover regressions on the higher-level requirement

Yes, we would like end-to-end tests through all affected high-level clients as well.

[mattsb42-aws]

I haven't gone through the tests yet, but wanted to get these notes out since I'll be busy for a while today.

[woodyza]

Sweet thanks @mattsb42-aws - I'll get onto these later on today.

Btw any pointers toward where the higher-level tests would best be implemented? I'm still figuring out the structure of the tests and how they interact with tox and the different test runs. Bit of a tox n00b tbh, don't spend a lot of time on library code that needs to work on more than one python version or platform.

[mattsb42-aws]

Sweet thanks @mattsb42-aws - I'll get onto these later on today.

Btw any pointers toward where the higher-level tests would best be implemented? I'm still figuring out the structure of the tests and how they interact with tox and the different test runs. Bit of a tox n00b tbh, don't spend a lot of time on library code that needs to work on more than one python version or platform.

The categories are described here[1]. Notably:

• functional: mark test as a functional test (does not require network access)
• integ: mark a test as an integration test (requires network access)
• ddb_integ: mark a test as an integration test that requires a real DynamoDB table (requires network access and not used in CI)

What I would have in mind for end-to-end tests for this behavior would be functional tests that mock out the DDB client. Unless you can figure out how to get moto to not process certain items, in which case functional tests that use that.

[1] https://github.com/aws/aws-dynamodb-encryption-python/blob/master/setup.cfg#L17-L30

[woodyza]

@mattsb42-aws I think I've tackled all your comments so far, I'm just jumping into the e2e tests now.

[mattsb42-aws]

Minor formatting fix that autoformat will take care of. Other than that, LGTM.

[mattsb42-aws]

Thanks of the contribution! I'll update #91 and #92 once we have this published in a new version.

[woodyza]

No probs! Thanks for the patient reviewing 😄

[mattsb42-aws]

@woodyza 1.1.0 is now live with these changes.