-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat!: Closure-based reader-writer serde for JSON, FormURL #1439
Changes from 1 commit
de363f2
214e5f7
e94c729
c192aaf
9c738c3
2575369
3316fab
b434813
2e148ce
e312db9
62a2cd2
6da7dca
91d8a76
a7d0436
078cbfe
045fc32
67df793
a4a06cc
3f75173
9667f31
05f3110
2a2382f
f59f1f4
a48de94
075ae45
37c4bd3
00e5feb
0c8fbcc
bd98fab
8f39040
785af50
2b4b3a5
97d840d
7a379c7
3c39ef3
32ee5c9
2268700
82d1da2
6de7ab7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,11 +191,13 @@ func addProtocolTests() { | |
let name: String | ||
let sourcePath: String | ||
let testPath: String? | ||
let buildOnly: Bool | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just generated from the Package.Base.swift above. |
||
|
||
init(name: String, sourcePath: String, testPath: String? = nil) { | ||
init(name: String, sourcePath: String, testPath: String? = nil, buildOnly: Bool = false) { | ||
self.name = name | ||
self.sourcePath = sourcePath | ||
self.testPath = testPath | ||
self.buildOnly = buildOnly | ||
} | ||
} | ||
|
||
|
@@ -217,21 +219,22 @@ func addProtocolTests() { | |
.init(name: "S3TestSDK", sourcePath: "\(baseDir)/s3"), | ||
.init(name: "rest_json_extras", sourcePath: "\(baseDirLocal)/rest_json_extras"), | ||
.init(name: "AwsQueryExtras", sourcePath: "\(baseDirLocal)/AwsQueryExtras"), | ||
.init(name: "EventStream", sourcePath: "\(baseDirLocal)/EventStream", buildOnly: true), | ||
.init(name: "RPCEventStream", sourcePath: "\(baseDirLocal)/RPCEventStream", buildOnly: true), | ||
.init(name: "Waiters", sourcePath: "\(baseDirLocal)/Waiters", testPath: "codegen/protocol-test-codegen-local/Tests"), | ||
] | ||
for protocolTest in protocolTests { | ||
package.targets += [ | ||
.target( | ||
name: protocolTest.name, | ||
dependencies: [.clientRuntime, .awsClientRuntime], | ||
path: "\(protocolTest.sourcePath)/swift-codegen/\(protocolTest.name)" | ||
), | ||
.testTarget( | ||
name: "\(protocolTest.name)Tests", | ||
dependencies: [.smithyTestUtils, .byNameItem(name: protocolTest.name, condition: nil)], | ||
path: "\(protocolTest.testPath ?? protocolTest.sourcePath)/swift-codegen/\(protocolTest.name)Tests" | ||
) | ||
] | ||
let target = Target.target( | ||
name: protocolTest.name, | ||
dependencies: [.clientRuntime, .awsClientRuntime], | ||
path: "\(protocolTest.sourcePath)/swift-codegen/\(protocolTest.name)" | ||
) | ||
let testTarget = protocolTest.buildOnly ? nil : Target.testTarget( | ||
name: "\(protocolTest.name)Tests", | ||
dependencies: [.smithyTestUtils, .byNameItem(name: protocolTest.name, condition: nil)], | ||
path: "\(protocolTest.testPath ?? protocolTest.sourcePath)/swift-codegen/\(protocolTest.name)Tests" | ||
) | ||
package.targets += [target, testTarget].compactMap { $0 } | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,8 +41,12 @@ val codegenTests = listOf( | |
"Waiters" | ||
), | ||
CodegenTest( | ||
"aws.protocoltests.restjson#TestService", | ||
"aws.protocoltests.eventstream#TestService", | ||
"EventStream" | ||
), | ||
CodegenTest( | ||
"aws.protocoltests.eventstream#RPCTestService", | ||
"RPCEventStream" | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code generate the non-RPC EventStream & RPC EventStream models with |
||
) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
namespace aws.protocoltests.eventstream | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is the same as |
||
|
||
use aws.protocols#awsJson1_1 | ||
use aws.api#service | ||
use aws.auth#sigv4 | ||
|
||
@awsJson1_1 | ||
@sigv4(name: "rpc-event-stream-test") | ||
@service(sdkId: "RPCEventStreamTest") | ||
service RPCTestService { version: "123", operations: [TestStreamOp] } | ||
|
||
@http(method: "POST", uri: "/test") | ||
operation TestStreamOp { | ||
input: TestStreamInputOutput, | ||
output: TestStreamInputOutput, | ||
errors: [SomeError], | ||
} | ||
|
||
structure TestStreamInputOutput { | ||
@httpPayload | ||
@required | ||
value: TestStream | ||
} | ||
|
||
@error("client") | ||
structure SomeError { | ||
Message: String, | ||
} | ||
|
||
union TestUnion { | ||
Foo: String, | ||
Bar: Integer, | ||
} | ||
|
||
structure TestStruct { | ||
someString: String, | ||
someInt: Integer, | ||
} | ||
|
||
structure MessageWithBlob { @eventPayload data: Blob } | ||
|
||
structure MessageWithString { @eventPayload data: String } | ||
|
||
structure MessageWithStruct { @eventPayload someStruct: TestStruct } | ||
|
||
structure MessageWithUnion { @eventPayload someUnion: TestUnion } | ||
|
||
structure MessageWithHeaders { | ||
@eventHeader blob: Blob, | ||
@eventHeader boolean: Boolean, | ||
@eventHeader byte: Byte, | ||
@eventHeader int: Integer, | ||
@eventHeader long: Long, | ||
@eventHeader short: Short, | ||
@eventHeader string: String, | ||
@eventHeader timestamp: Timestamp, | ||
} | ||
structure MessageWithHeaderAndPayload { | ||
@eventHeader header: String, | ||
@eventPayload payload: Blob, | ||
} | ||
structure MessageWithNoHeaderPayloadTraits { | ||
someInt: Integer, | ||
someString: String, | ||
} | ||
|
||
structure MessageWithUnboundPayloadTraits { | ||
@eventHeader header: String, | ||
unboundString: String, | ||
} | ||
|
||
@streaming | ||
union TestStream { | ||
MessageWithBlob: MessageWithBlob, | ||
MessageWithString: MessageWithString, | ||
MessageWithStruct: MessageWithStruct, | ||
MessageWithUnion: MessageWithUnion, | ||
MessageWithHeaders: MessageWithHeaders, | ||
MessageWithHeaderAndPayload: MessageWithHeaderAndPayload, | ||
MessageWithNoHeaderPayloadTraits: MessageWithNoHeaderPayloadTraits, | ||
MessageWithUnboundPayloadTraits: MessageWithUnboundPayloadTraits, | ||
SomeError: SomeError, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
namespace aws.protocoltests.restjson | ||
namespace aws.protocoltests.eventstream | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the non-RPC EventStream model. It is pre-existing, and was added when event streaming was first added to the SDK. |
||
use aws.protocols#restJson1 | ||
use aws.api#service | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package software.amazon.smithy.aws.swift.codegen.message | |
import software.amazon.smithy.codegen.core.CodegenException | ||
import software.amazon.smithy.codegen.core.Symbol | ||
import software.amazon.smithy.model.shapes.MemberShape | ||
import software.amazon.smithy.model.shapes.Shape | ||
import software.amazon.smithy.model.shapes.ShapeType | ||
import software.amazon.smithy.model.shapes.UnionShape | ||
import software.amazon.smithy.model.traits.EventHeaderTrait | ||
|
@@ -14,6 +13,7 @@ import software.amazon.smithy.swift.codegen.SwiftWriter | |
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator | ||
import software.amazon.smithy.swift.codegen.integration.serde.readwrite.NodeInfoUtils | ||
import software.amazon.smithy.swift.codegen.integration.serde.readwrite.WritingClosureUtils | ||
import software.amazon.smithy.swift.codegen.integration.serde.readwrite.requestWireProtocol | ||
import software.amazon.smithy.swift.codegen.integration.serde.readwrite.responseWireProtocol | ||
import software.amazon.smithy.swift.codegen.integration.serde.struct.writerSymbol | ||
import software.amazon.smithy.swift.codegen.model.eventStreamEvents | ||
|
@@ -70,7 +70,19 @@ class MessageMarshallableGenerator( | |
eventPayloadBinding != null -> renderSerializeEventPayload(ctx, eventPayloadBinding, writer) | ||
unbound.isNotEmpty() -> { | ||
writer.addStringHeader(":content-type", payloadContentType) | ||
renderPayloadSerialization(ctx, writer, variant) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added support here for unbound event members, which (according to the Smithy spec) are supposed to be serialized by
(https://smithy.io/2.0/spec/streaming.html#event-message-serialization) So, I do that here. |
||
writer.addImport(ctx.service.writerSymbol.namespace) | ||
val nodeInfo = NodeInfoUtils(ctx, writer, ctx.service.requestWireProtocol).nodeInfo(member, true) | ||
writer.write("let writer = \$N(nodeInfo: \$L)", ctx.service.writerSymbol, nodeInfo) | ||
unbound.forEach { | ||
val writingClosure = WritingClosureUtils(ctx, writer).writingClosure(ctx.model.expectShape(it.target)) | ||
writer.write( | ||
"try writer[\$S].write(value.\$L, with: \$L)", | ||
it.memberName, | ||
ctx.symbolProvider.toMemberName(it), | ||
writingClosure, | ||
) | ||
} | ||
writer.write("payload = try writer.data()") | ||
} | ||
} | ||
writer.dedent() | ||
|
@@ -108,7 +120,7 @@ class MessageMarshallableGenerator( | |
} | ||
ShapeType.STRUCTURE, ShapeType.UNION -> { | ||
writer.addStringHeader(":content-type", payloadContentType) | ||
renderPayloadSerialization(ctx, writer, target) | ||
renderPayloadSerialization(ctx, writer, member) | ||
} | ||
else -> throw CodegenException("unsupported shape type `${target.type}` for target: $target; expected blob, string, structure, or union for eventPayload member: $member") | ||
} | ||
|
@@ -175,14 +187,16 @@ class MessageMarshallableGenerator( | |
write("headers.append(.init(name: \$S, value: .string(\$S)))", name, value) | ||
} | ||
|
||
private fun renderPayloadSerialization(ctx: ProtocolGenerator.GenerationContext, writer: SwiftWriter, shape: Shape) { | ||
private fun renderPayloadSerialization(ctx: ProtocolGenerator.GenerationContext, writer: SwiftWriter, memberShape: MemberShape) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed this to render serialization for the member shape so that other member info is available, not just the target shape. |
||
// get a payload serializer for the given members of the variant | ||
val nodeInfoUtils = NodeInfoUtils(ctx, writer, ctx.service.responseWireProtocol) | ||
val rootNodeInfo = nodeInfoUtils.nodeInfo(shape) | ||
val valueWritingClosure = WritingClosureUtils(ctx, writer).writingClosure(shape) | ||
val rootNodeInfo = nodeInfoUtils.nodeInfo(memberShape, true) | ||
val valueWritingClosure = WritingClosureUtils(ctx, writer).writingClosure(memberShape) | ||
writer.addImport(ctx.service.writerSymbol.namespace) | ||
writer.write( | ||
"payload = try \$N.write(value, rootNodeInfo: \$L, with: \$L)", | ||
"payload = try \$N.write(value.\$L, rootNodeInfo: \$L, with: \$L)", | ||
ctx.service.writerSymbol, | ||
ctx.symbolProvider.toMemberName(memberShape), | ||
rootNodeInfo, | ||
valueWritingClosure, | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,7 +73,7 @@ class MessageUnmarshallableGenerator(val ctx: ProtocolGenerator.GenerationContex | |
writer.indent { | ||
writer.write("switch params.exceptionType {") | ||
streamShape.eventStreamErrors(ctx.model).forEach { member -> | ||
writer.write("case \"${member.memberName}\":") | ||
writer.write("case \$S:", member.memberName) | ||
writer.indent { | ||
renderReadToValue(writer, member) | ||
writer.write("return value") | ||
|
@@ -170,7 +170,7 @@ class MessageUnmarshallableGenerator(val ctx: ProtocolGenerator.GenerationContex | |
// and then assign each deserialized payload member to the current builder instance | ||
unbound.forEach { | ||
renderReadToValue(writer, it) | ||
writer.write("event.\$L = value", memberName) | ||
writer.write("event.\$L = value", ctx.symbolProvider.toMemberName(it)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change needed to reference the correct member name. |
||
} | ||
} | ||
} | ||
|
@@ -190,13 +190,14 @@ class MessageUnmarshallableGenerator(val ctx: ProtocolGenerator.GenerationContex | |
ShapeType.STRING -> writer.write("event.\$L = String(data: message.payload, encoding: .utf8)", memberName) | ||
ShapeType.STRUCTURE, ShapeType.UNION -> { | ||
renderReadToValue(writer, member) | ||
writer.write("event.\$L = value", memberName) | ||
writer.write("event.\$L = value", ctx.symbolProvider.toMemberName(member)) | ||
} | ||
else -> throw CodegenException("unsupported shape type `${target.type}` for target: $target; expected blob, string, structure, or union for eventPayload member: $member") | ||
} | ||
} | ||
|
||
private fun renderReadToValue(writer: SwiftWriter, memberShape: MemberShape) { | ||
writer.addImport(ctx.service.readerSymbol.namespace) | ||
val readingClosure = ReadingClosureUtils(ctx, writer).readingClosure(memberShape) | ||
writer.write( | ||
"let value = try \$N.readFrom(message.payload, with: \$L)", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,11 +29,11 @@ extension EventStreamTestClientTypes.TestStream { | |
case .messagewithstruct(let value): | ||
headers.append(.init(name: ":event-type", value: .string("MessageWithStruct"))) | ||
headers.append(.init(name: ":content-type", value: .string("application/json"))) | ||
payload = try SmithyJSON.Writer.write(value, rootNodeInfo: "TestStruct", with: EventStreamTestClientTypes.TestStruct.write(value:to:)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Next few files are codegen test updates. |
||
payload = try SmithyJSON.Writer.write(value.someStruct, rootNodeInfo: "", with: EventStreamTestClientTypes.TestStruct.write(value:to:)) | ||
case .messagewithunion(let value): | ||
headers.append(.init(name: ":event-type", value: .string("MessageWithUnion"))) | ||
headers.append(.init(name: ":content-type", value: .string("application/json"))) | ||
payload = try SmithyJSON.Writer.write(value, rootNodeInfo: "TestUnion", with: EventStreamTestClientTypes.TestUnion.write(value:to:)) | ||
payload = try SmithyJSON.Writer.write(value.someUnion, rootNodeInfo: "", with: EventStreamTestClientTypes.TestUnion.write(value:to:)) | ||
case .messagewithheaders(let value): | ||
headers.append(.init(name: ":event-type", value: .string("MessageWithHeaders"))) | ||
if let headerValue = value.blob { | ||
|
@@ -70,14 +70,19 @@ extension EventStreamTestClientTypes.TestStream { | |
case .messagewithnoheaderpayloadtraits(let value): | ||
headers.append(.init(name: ":event-type", value: .string("MessageWithNoHeaderPayloadTraits"))) | ||
headers.append(.init(name: ":content-type", value: .string("application/json"))) | ||
payload = try SmithyJSON.Writer.write(value, rootNodeInfo: "MessageWithNoHeaderPayloadTraits", with: EventStreamTestClientTypes.MessageWithNoHeaderPayloadTraits.write(value:to:)) | ||
let writer = SmithyJSON.Writer(nodeInfo: "") | ||
try writer["someInt"].write(value.someInt, with: Swift.Int.write(value:to:)) | ||
try writer["someString"].write(value.someString, with: Swift.String.write(value:to:)) | ||
payload = try writer.data() | ||
case .messagewithunboundpayloadtraits(let value): | ||
headers.append(.init(name: ":event-type", value: .string("MessageWithUnboundPayloadTraits"))) | ||
if let headerValue = value.header { | ||
headers.append(.init(name: "header", value: .string(headerValue))) | ||
} | ||
headers.append(.init(name: ":content-type", value: .string("application/json"))) | ||
payload = try SmithyJSON.Writer.write(value, rootNodeInfo: "MessageWithUnboundPayloadTraits", with: EventStreamTestClientTypes.MessageWithUnboundPayloadTraits.write(value:to:)) | ||
let writer = SmithyJSON.Writer(nodeInfo: "") | ||
try writer["unboundString"].write(value.unboundString, with: Swift.String.write(value:to:)) | ||
payload = try writer.data() | ||
case .sdkUnknown(_): | ||
throw ClientRuntime.ClientError.unknownError("cannot serialize the unknown event type!") | ||
} | ||
|
@@ -161,7 +166,7 @@ extension EventStreamTestClientTypes.TestStream { | |
event.header = value | ||
} | ||
let value = try SmithyJSON.Reader.readFrom(message.payload, with: Swift.String.read(from:)) | ||
event.messagewithunboundpayloadtraits = value | ||
event.unboundString = value | ||
return .messagewithunboundpayloadtraits(event) | ||
default: | ||
return .sdkUnknown("error processing event stream, unrecognized event: \(params.eventType)") | ||
|
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.
Added the ability to add these two Smithy service definitions to local protocol tests.
These two Smithy services don't have any protocol tests, but at least this will build them & verify that the code compiles.
Had to add a little logic here to allow adding protocol test targets to the manifest that don't have actual test bundles.