-
Notifications
You must be signed in to change notification settings - Fork 321
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
Add support for jackson field ids #868
base: main
Are you sure you want to change the base?
Conversation
msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java
Outdated
Show resolved
Hide resolved
@komamitsu @xerial Can CI be run on this PR to show the failing test? Then I'll follow-up with the implementation changes. |
@komamitsu @xerial Another CI run please 🙏 |
🔨 Build failed with test showcasing new edge case.
Will now demonstrate passing test with implementation changes. |
@komamitsu @xerial This is ready for another CI run to showcase the new test passing. |
@Override | ||
public void writeFieldId(long id) throws IOException | ||
{ | ||
addKeyNode(id); |
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.
@komamitsu @xerial For backwards compatibility, I imagine we need some sort of feature flag that defaults to the prior implementation. Do you guys have any suggestions for how that flag should be defined? Should I just use the MessagePack.PackerConfig
, or should I pass it directly from the MessagePackFactory
to the MessagePackGenerator
?
addKeyNode(id); | |
if (writeIntegerMapKeysAsStringKeys) { | |
super.writeFieldId(id); | |
} else { | |
addKeyNode(id); | |
} |
Background
Jackson core has interfaces for field ids. This is a great opportunity for msgpack, since the protocol allows for integer keys, enabling more advanced binary serialization strategies for further reduced message size.
Current implementation of msgpack-jackson only allows for coercing strings to integers. Implementing the formal interfaces will enable end-to-end map serialization with mixed string/integer keys. Other msgpack implementations already support this.
Summary of Changes
KeyDeserializer
to deserialize field ids.