Add useDeterministicOrdering to JSONEncodingOptions
#1478
Conversation
Adds an option to ensure that JSON serialization is deterministic when serializing Protobuf `map` fields. This should be the only type that needs to be sorted, since individual fields are serialized in order by the generated code that invokes `try visitor.visit(...)` for each field. Resolves apple#1477
|
@thomasvl would you mind taking a look at this? 🙏🏽 |
Implements the same setting added for JSON in apple#1478 for binary serialization. Related to apple#1477.
Implements the same setting added for JSON in apple#1478 for binary serialization. Related to apple#1477.
This reverts commit 1ea1a8a.
Sorry, been OOO for a bit, seems like these PR and issue got some discussion. Will catch up and hopefully some of the Apple folks will take a look. If these PRs do land, there's also the question of it they can/should go back to the 1.x branch/release or just sit awaiting a 2.0 release. |
Thanks @thomasvl.
These are all backward-compatible and opt-in, so I think they should be okay to include in 1.x (not sure what the release plan for 2.0 is). |
Since it is a new Option, it is sorta an ABI change, but I think the past stance has been these sorta things are ok. That's where Apple (as repo owner) gets the final call. |
IMO this is fine to add from just a pure API/ABI compatibility perspective. We are just adding a new field to a non-frozen struct. On the overall feature, I am +1. If other languages are providing a similar option I don't see why we shouldn't and the documentation calls out that this is unstable and break over versions. |
|
For the record, I'm happy with the intent here. Adding a new field to a non-frozen struct is forward-compatible API and ABI. (Old client source/binaries work correctly with new library.) So back-porting to 1.x should be fine. I'll do another skim over the source to see if I have any last-minute nits... |
|
Thank you! |
|
Thank you both! |
* Add `useDeterministicOrdering` to `JSONEncodingOptions` Adds an option to ensure that JSON serialization is deterministic when serializing Protobuf `map` fields. This should be the only type that needs to be sorted, since individual fields are serialized in order by the generated code that invokes `try visitor.visit(...)` for each field. Fixes apple#1477
…pple#1480) * Add `BinaryEncodingOptions` & option for deterministic ordering Implements the same setting added for JSON in apple#1478 for binary serialization. Related to apple#1477.
## Summary Implements support for Connect unary GET requests per https://connectrpc.com/docs/protocol#unary-get-request. By default, this behavior is disabled and can be enabled through an option on `ProtocolClientConfig` (as demonstrated in the Eliza app). Behavior is exercised in a new conformance test, but I do intend to add more unit tests for this after #208. ## Linked issues - Requires apple/swift-protobuf#1487 - Related to apple/swift-protobuf#1478 - Related to apple/swift-protobuf#1480 - Resolves #196
Adds an option to ensure that JSON serialization is deterministic when serializing Protobuf
mapfields. This should be the only type that needs to be sorted, since individual fields are serialized in order by the generated code that invokestry visitor.visit(...)for each field.In order to avoid the overhead of sorting for the default case, there are no changes to the existing serialization strategy when this setting is disabled.
This new option is similar to what Protobuf provides in Go, and the docstring added in this PR matches that.
Resolves #1477