Conversation
de98be8 to
6976217
Compare
6976217 to
294e2cd
Compare
|
|
||
| namespace { | ||
|
|
||
| class Person : public SerializableStruct { |
There was a problem hiding this comment.
at my desk i was going "yes, yes, yes, yes, yes, exactly" 💯
a885d30 to
755dd92
Compare
a810118 to
b877f00
Compare
b877f00 to
43109f8
Compare
…nd ClientProtocol
… with nested-shape codegen blueprint tests
…erde ClientProtocol
43109f8 to
229aa7f
Compare
| Aws::String GetProtocolId() const override; | ||
| Aws::String GetContentType() const override; | ||
| SerializerOutcome SerializeInput(const Schema& schema, const SerializableStruct& input) const override; | ||
| Aws::UniquePtr<ShapeDeserializer> CreateOutputDeserializer(const unsigned char* data, size_t length) const override; |
There was a problem hiding this comment.
not absolutely needed on this, but lets talk about the API
const unsigned char* data, size_t length
this specific very common and normal api has caused a substantial amount of pain. so much so that c++ introduced a library type called std::span. The CRT also has the same idea as aws_byte_cursor. it might be worth seeing if you can use ByteCursor instead of decomposition of it.
There was a problem hiding this comment.
switched to Aws::Crt::ByteCursor throughout. Codec::CreateDeserializer/DeserializeShape, ClientProtocol::CreateOutputDeserializer/DeserializeResponse, and the three {Json,Xml,Cbor}ShapeDeserializer ctors now take a single ByteCursor instead of (const unsigned char*, size_t)
|
|
||
| // Sketch of a generated, schema-driven shape: it owns its Schema, writes its | ||
| // members (push), and populates itself from per-member deserialize callbacks. | ||
| class Widget : public SerializableStruct { |
There was a problem hiding this comment.
is there a test like this for XML? i didnt see one, and i think theres def value in having the full "fake codegen expierence tests" for each deserializer
There was a problem hiding this comment.
ya XML and CBOR didn't have one, only JSON did, I've added DeserializesNestedIntoClass to both XmlShapeDeserializerTest and CborShapeDeserializerTest
| if (!token.has_value()) { | ||
| return {}; | ||
| } | ||
| DateTime parsed(*token, DateFormat::ISO_8601); |
There was a problem hiding this comment.
for both read and write timestamps on all codecs does hardcoding ISO_8601 work? i think this would break both in the read and write path where timestampFormat is a trait on the shape. is there protocol tests around this?
There was a problem hiding this comment.
it was hardcoded and would break any shape overriding the format. TimestampFormatTrait existed but nothing read it. Fixed in 9caf95f
| Aws::Crt::Optional<Aws::String> ReadString(const Schema& schema) override; | ||
| Aws::Crt::Optional<Aws::Utils::DateTime> ReadTimestamp(const Schema& schema) override; | ||
| Aws::Crt::Optional<Aws::Utils::ByteBuffer> ReadBlob(const Schema& schema) override; | ||
| Aws::Crt::Optional<int> ReadEnum(const Schema& schema) override; |
There was a problem hiding this comment.
I know this is late in the game, but should ReadEnum return a int? also should ReadEnum exist? smithy java doesnt have it. since enums are represented as strings on the wire. shouldnt we keep them as string, and allow enums to interpret that value?
There was a problem hiding this comment.
removing WriteEnum/ReadEnum
…fake-codegen deser tests, remove enum methods
This branch has not been deployed
2 participants
Adds response deserialization and the protocol-selection layer to the schema-serde runtime.
ShapeDeserializerreworked to a push/consumer model (ReadStruct/ReadList/ReadMap), the dual ofShapeSerializer; scalars take const Schema&.JsonShapeDeserializerandXmlShapeDeserializer(new),CborShapeDeserializerupdated to match.QueryShapeSerializer: full nesting +ec2Queryflavor (newEc2QueryNameTrait).Codec(Json/Xml/Cbor) pairs serializer+deserializer;ClientProtocolpicks codec + content-type per protocol (restJson1, awsJson1.0/1.1, rpcv2Cbor, restXml, awsQuery, ec2Query),including the asymmetric query request/XML response case.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.