Schema types and ShapeSerializer interface#3837
Conversation
| private: | ||
| void WriteKey(const chat* key); | ||
|
|
||
| Aws::Crt::Cbor::CborEncoder m_encoder; |
There was a problem hiding this comment.
so this would be a prime example to use PIMPL to avoid including a crt header, i.e
class AWS_CORE_API CborShapeSerializer {
public:
// ...
private:
struct CborEncoderImpl;
std::unique_ptr<CborEncoderImpl> impl_;
}then in the yet to be defined implementation you can do
struct CborShapeSerializer::CborEncoderImpl {
Aws::Crt::Cbor::CborEncoder m_encoder;
};this way consumers don't have a direct dependency on the CRT via the header
|
|
||
| Aws::Utils::Json::JsonValue GetResult() const { return m_root; } | ||
| Aws::String GetPayload() const; | ||
| private: |
There was a problem hiding this comment.
same as cbor, lets make this PIMPL, should have one member that is a pointer, then in the .cpp define the actual implementation of it
| Aws::Utils::Array<Aws::Utils::Json::JsonValue> listItems; | ||
| size_t listIndex; | ||
| }; | ||
| Aws::Vector<StackEntry> m_stack; |
There was a problem hiding this comment.
this scares me a little bit but we can wait for a full implementation review. what is the stack used for, are we keeping a in memory representation of the json object? keeping a in memory representation of the object is something that we want to avoid that came up as we profiled this. we should be pre-allocating objects as well so that we dont pay a hefty alloc per object added to the stack
| Document | ||
| }; | ||
|
|
||
| enum class TimestampFormate : uint8_t { |
There was a problem hiding this comment.
nit: typo s/Formate/Format/g
| }; | ||
|
|
||
| enum class FieldLocation : uint8_t { | ||
| Bode, |
| @@ -0,0 +1,3 @@ | |||
| // | |||
There was a problem hiding this comment.
lets remove this file until we actually use it
| StatusCode | ||
| }; | ||
|
|
||
| struct Schema; |
There was a problem hiding this comment.
so in the original design we only had Schema and that looked roughly like
class Schema { // Schema.h — lightweight shape descriptor
public:
ShapeId getId() const;
ShapeType getType() const;
Schema getMember(const char* name) const;
Schema getMember(int position) const;
// traits, member iteration, etc.
};
where a serializer would interact with it like
class ShapeSerializer { // ShapeSerializer.h — abstract, format-agnostic
public:
virtual void writeString(const Schema& schema, const Aws::String& value) = 0;
virtual void writeInteger(const Schema& schema, int value) = 0;
virtual void writeStruct(const Schema& schema, const SerializableShape& value) = 0;
virtual void writeMap(const Schema& schema, ...) = 0;
virtual Aws::String flush() = 0;
};
why do we now have two types FieldSchema and Schema? lets just copye how java does it where all of their serializers take a schema object, for now, for this PR that schema object can be empty, and we can work out the implementation when we add out first implementation
sbiscigl
left a comment
sbiscigl
left a comment
There was a problem hiding this comment.
approved, address my comment in the next iteration nsince this is very "define interfaces" step
| class AWS_CORE_API CborShapeSerializer : public ShapeSerializer { | ||
| public: | ||
| CborShapeSerializer(); | ||
| ~CborShapeSerializer(); |
There was a problem hiding this comment.
destructors need to be virtual or inheritance needs to be final
There was a problem hiding this comment.
making the inheritance final
2 participants
Initial scaffolding for schema-based serialization adds the core Schema/FieldSchema data types and ShapeSerializer interface
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.