Support schema evolution when using non-default constructors#360
Open
nicodeslandes wants to merge 8 commits intoch-robinson:mainfrom
Open
Support schema evolution when using non-default constructors#360nicodeslandes wants to merge 8 commits intoch-robinson:mainfrom
nicodeslandes wants to merge 8 commits intoch-robinson:mainfrom
Conversation
deserializers for record The constructor candidates are sorted by best fit, and the top scorer is selected Add test for custom record deserializer to ensure backward compatibility Fixed json deserialization of dynamic types Handle case where no suitable constructor can be found to generate a deserializer
a715434 to
fc7681c
Compare
Contributor
Author
|
I just rebased over the latest changes from main. This is now ready to be merged (at least as far as git is concerned :-) ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Support new fields in Schema when using non-default constructors
Description
This PR addresses #358 by making the record deserializer more tolerant when selecting constructors, which is critical for supporting schema evolution in immutable records.
The Problem
Previously,
BinaryRecordDeserializerBuilder(and the JSON equivalent) required a constructor to match the Avro schema fields exactly. This meant that adding a new field to a schema (typically a backward-compatible change) would break deserialization for existing C#recordtypes that hadn't yet been updated to include the new field. The deserializer would fail to find a matching constructor and throw an exception.This is rather inconsistent with the way we handle new properties for classes that expose public properties and a default constructor
Example:
Old schema:
{ "type": "record", "name": "Person", "fields": [ {"name": "name", "type": "string"} ] }C# record / class:
All good so far, we can happilly serialize/deserialize to/from either form of DTO
But when a new, backward-compatible field is added to the schema::
{ "type": "record", "name": "Person", "fields": [ {"name": "name", "type": "string"}, {"name": "age", "type": "int", "default": 20} ] }PersonClassis still usable; we can generate both serializer and deserializer for itBut attempting to generate a deserializer for
PersonRecordproduces the following exception:The Fix
The deserializer logic has been updated to select the "best match" constructor rather than requiring an exact match:
This behavior allows a client with an older version of a
record(missing a new field) to successfully deserialize data from a newer schema version, ignoring the unknown fields.Additional Improvements
Verification
New tests in
PartialConstructorMatchTestsverify: