Skip to content

Schema types and ShapeSerializer interface#3837

Open
pulimsr wants to merge 3 commits into
mainfrom
schema-serde
Open

Schema types and ShapeSerializer interface#3837
pulimsr wants to merge 3 commits into
mainfrom
schema-serde

Conversation

@pulimsr
Copy link
Copy Markdown
Collaborator

@pulimsr pulimsr commented Jun 2, 2026

Initial scaffolding for schema-based serialization adds the core Schema/FieldSchema data types and ShapeSerializer interface

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

private:
void WriteKey(const chat* key);

Aws::Crt::Cbor::CborEncoder m_encoder;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo s/Formate/Format/g

};

enum class FieldLocation : uint8_t {
Bode,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: Node?

@@ -0,0 +1,3 @@
//
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets remove this file until we actually use it

StatusCode
};

struct Schema;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@pulimsr pulimsr marked this pull request as ready for review June 3, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants