Handle JSON null token when reading collections mapped to JSON#38459
Handle JSON null token when reading collections mapped to JSON#38459Copilot wants to merge 5 commits into
Conversation
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes JSON primitive-collection reader/writers so that a top-level JSON null token materializes as a null collection rather than throwing, aligning behavior across direct-reader usage (e.g., value converters) and nested JSON reads.
Changes:
- Updated
JsonCollectionOfReferencesReaderWriterandJsonCollectionOfStructsReaderWriterto returnnullwhen the current token isJsonTokenType.Null. - Updated
JsonCollectionOfNullableStructsReaderWriterto returnnullwhen the current token isJsonTokenType.Null(after the existingNoneadvance). - Added
JsonCollectionReaderWriterTestto assert"null"round-trips tonullacross reference, struct, and nullable-struct collection reader/writers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/EFCore/Storage/Json/JsonCollectionOfReferencesReaderWriter.cs | Accept top-level Null token and return null rather than throwing. |
| src/EFCore/Storage/Json/JsonCollectionOfStructsReaderWriter.cs | Accept top-level Null token and return null rather than throwing. |
| src/EFCore/Storage/Json/JsonCollectionOfNullableStructsReaderWriter.cs | Accept top-level Null token and return null (with existing None handling). |
| test/EFCore.Tests/Storage/Json/JsonCollectionReaderWriterTest.cs | New unit tests validating "null" reads as null for various collection shapes. |
| if (manager.CurrentReader.TokenType == JsonTokenType.Null) | ||
| { | ||
| return null!; | ||
| } |
| /// <inheritdoc /> | ||
| public override object FromJsonTyped(ref Utf8JsonReaderManager manager, object? existingObject = null) | ||
| { | ||
| if (manager.CurrentReader.TokenType == JsonTokenType.Null) |
There was a problem hiding this comment.
@copilot Try find a way to short-circuit this at a different level so that this doesn't need to be handled in each ReaderWriter
There was a problem hiding this comment.
Done. The null short-circuit is now in JsonValueReaderWriter.FromJsonString — when the token type is JsonTokenType.Null it returns null directly, before calling any per-type FromJson. The return type was also updated from object to object? and the API baseline updated accordingly. The three collection reader/writers no longer need individual null checks.
Commit: 7e0c0b2
|
|
||
| namespace Microsoft.EntityFrameworkCore.Storage; | ||
|
|
||
| public class JsonCollectionReaderWriterTest |
There was a problem hiding this comment.
@copilot Add tests to JsonTypesTestBase for these scenarios instead of these unit tests
There was a problem hiding this comment.
Done. The unit test file has been removed and three new test methods have been added to JsonTypesTestBase: Can_read_write_null_value_of_collection_of_string_JSON_values, Can_read_write_null_value_of_collection_of_int_JSON_values, and Can_read_write_null_value_of_collection_of_nullable_int_JSON_values. The Can_read_and_write_JSON_value helper was also updated to exercise FromJsonString("null") for null values.
Commit: 7e0c0b2
…aderWriter Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
A JSON column/property mapped to a primitive collection (e.g.
List<string>?) threwInvalidOperationException: Invalid token type: 'Null'when the stored value was a JSONnullliteral instead of an array.The three collection reader/writers only accepted a
StartArraytoken at the top level. Nulls are usually short-circuited before reaching the reader (e.g.FromJsonPropertyString), but the value-converter path (CollectionToJsonStringConverter.FromJsonString) and nested JSON documents can drive the reader directly to aNulltoken.Changes
JsonValueReaderWriter.FromJsonString: short-circuits on a top-levelJsonTokenType.Nulland returnsnull— one central place covers all reader/writers without needing per-class handling. Return type updated fromobjecttoobject?to accurately reflect thatnullis a valid return value.EFCore.baseline.json: updatedFromJsonStringsignature toobject?.Can_read_write_null_value_of_collection_of_string_JSON_values,Can_read_write_null_value_of_collection_of_int_JSON_values, andCan_read_write_null_value_of_collection_of_nullable_int_JSON_valuestoJsonTypesTestBase, asserting that a JSON"null"value round-trips as anullcollection while normal arrays still read correctly. TheCan_read_and_write_JSON_valuehelper now also exercises theFromJsonString("null")path for null values.