fix: address various failing protocol tests #1223
Conversation
lauzadis
changed the title
lauzadis
added
the
no-changelog
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
lauzadis
changed the title
This comment has been minimized.
This comment has been minimized.
| if (builderMemberSymbol.shape is BlobShape && builderMemberSymbol.isNotNullable) { | ||
| writer.addImport(RuntimeTypes.Core.Text.Encoding.decodeBase64) | ||
| } |
There was a problem hiding this comment.
decodeBase64 is used in KotlinSymbolProvider, but we don't have access to the KotlinWriter there, so the import gets added here if necessary. Any other ideas to work around this?
There was a problem hiding this comment.
Correct, we don't have a KotlinWriter (nor should we) in KotlinSymbolProvider. But we're still providing symbols and symbols can declare additional imports. We should be setting up those references before returning from the symbol provider.
This probably means a bit of a light refactor in the symbol provider since it's mostly slinging text strings around and wrapping it into a symbol right before returning. But we probably need better awareness of non-textual symbol info in places like getDefaultValue.
There was a problem hiding this comment.
I did a minimal refactor to give getDefaultValue access to the Symbol being built, calling addReferences where appropriate
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| targetShape.isBlobShape -> "${node.toString().dq()}.encodeToByteArray()" | ||
| targetShape.isBlobShape -> "${node.toString().dq()}.decodeBase64().encodeToByteArray()" |
There was a problem hiding this comment.
Question: Don't we need this for streaming targets too? (i.e., the when branch above this one)
| if (builderMemberSymbol.shape is BlobShape && builderMemberSymbol.isNotNullable) { | ||
| writer.addImport(RuntimeTypes.Core.Text.Encoding.decodeBase64) | ||
| } |
There was a problem hiding this comment.
Correct, we don't have a KotlinWriter (nor should we) in KotlinSymbolProvider. But we're still providing symbols and symbols can declare additional imports. We should be setting up those references before returning from the symbol provider.
This probably means a bit of a light refactor in the symbol provider since it's mostly slinging text strings around and wrapping it into a symbol right before returning. But we probably need better awareness of non-textual symbol info in places like getDefaultValue.
Affected ArtifactsChanged in size
|
* fix: add 0.9.x aws-crt-kotlin transform (#1220) * fix: Ensure `Host` header is included when signing auth tokens (#1222) * chore: release 1.4.1 * chore: bump snapshot version to 1.4.2-SNAPSHOT * fix: address various failing protocol tests (#1223) * misc: re-enable `kotlinWarningsAsErrors=true` (#1224) * fix: ignore hop-by-hop headers when signing requests (#1227) * chore: release 1.4.2 * chore: bump snapshot version to 1.4.3-SNAPSHOT * misc: add telemetry configuration to DefaultAwsSigner (#1226) * add telemetry provider configuration * lint * address pr reviews * add changelog --------- Co-authored-by: Matas <[email protected]> Co-authored-by: aws-sdk-kotlin-ci <[email protected]> Co-authored-by: Ian Botsford <[email protected]>
NullAndEmptyHeadersServertests forrestJson1andrestXmlsmithy#2433 and serialize empty strings and lists to headers in restXml and restJson1 smithy#2403.merging an upstream fix to make Base64 string length a multiple of 4 (aka padded)adding support for unpadded Base64 inputsIssue #
Description of changes
This change is required to comply with Smithy protocol tests which assert that empty header values are sent.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.