Closed nicksloan closed 3 months ago
@adam-fowler Thanks for the feedback. The test definitely won’t pass. The inputs from the AWS docs don’t really work for a reproducible output. Working on a better test case.
You'll need to rebase 7.x.x to get this compiling again by the way
I have to approach testing this differently, but I have a plan. Update coming sometime tomorrow afternoon.
Why can't you use the example to verify you are calculating the correct signature?
The example string to sign in their docs is a base 64 encoded JSON object, but the JSON object they used has arbitrary whitespace in it, so it would be absurd to try to make this code produce the exact same string.
I can make use of the examples from the docs to test at a more granular level. If I test the getSignature method in isolation (rather than generatePresignedPost as a whole), I can supply the string to sign directly, rather than generating it. I’ll push an updated test that does that once I’m at my desk today.
The other thing I tried is using Boto3 to generate known to work values from example credentials, but Python preserves order in dictionaries, which means the properties of the JSON object end up in a different order. Same problem, we’d have to make our code worse to match the output exactly, for absolutely no gain outside of the test suite.
In addition to testing getSignature, I’ll test getPresignedPostCredential, and I’ll have to test that generatePresignedPost produces the right URL, and a fields dictionary and post policy that have all the right keys. If I test all of that, I’m confident that the code is well covered. I’m about halfway done with these tests, and should be able to finish this morning. Thanks for your patience.
@adam-fowler Ready for another review.
I've just run swift format on everything. And will merge when all checks are complete
Looks good Have you tested with real assets?
I did test with real assets and credentials, but not from outside of the package until today. Submitted a small follow-up PR and have confirmed that everything works as intended once that is merged.
Adds a generatePresignedPost method to the S3 service, similar to what is available in Boto3.
Amazon docs for this type of request live here.
Remaining work: