Add JsonConverter classes for Date and Time to support System.Text.Json deserialization#641
Add JsonConverter classes for Date and Time to support System.Text.Json deserialization#641
Conversation
…ation Co-authored-by: gavinbarron <7122716+gavinbarron@users.noreply.github.com>
|
@copilot please run dotnet format |
Co-authored-by: gavinbarron <7122716+gavinbarron@users.noreply.github.com>
|
baywet
left a comment
There was a problem hiding this comment.
One of the key design principles of abstractions is to NOT be tied to any serialization format.
These changes completely disregard that aspect.
The OP suggested two other approaches: JsonConverter, or a setter/init.
I'm in favour of the converter approach.
|
Thanks @baywet I was unaware of that design goal. and with that in mind yes, the converter is the best approach here. |
|
@copilot Let's change direction and instead of using a [JsonConstructor] let's provide a JsonConverter for each of these types |
Co-authored-by: gavinbarron <7122716+gavinbarron@users.noreply.github.com>
Done! Changed to use JsonConverter approach in commit aa05ced. The converters are now in the serialization/json project, keeping abstractions free of serialization dependencies. Users can access them via |
| @@ -34,4 +35,24 @@ namespace Microsoft.Kiota.Serialization.Json; | |||
| [JsonSerializable(typeof(Date?))] | |||
| [JsonSerializable(typeof(Time))] | |||
| [JsonSerializable(typeof(Time?))] | |||
There was a problem hiding this comment.
I'm curious: should these attributes be updated to point to the converters as well?



Fix Date and Time deserialization with System.Text.Json ✅
Summary
Successfully fixed the deserialization issue with
DateandTimetypes when usingSystem.Text.Json.JsonSerializerby providing JsonConverter classes in the serialization package, keeping the abstractions project free of serialization dependencies.Approach
Based on maintainer feedback, the solution uses JsonConverter classes instead of JsonConstructor attributes to maintain the design principle that abstractions should NOT be tied to any serialization format.
Changes Made:
Serialization Package (
Microsoft.Kiota.Serialization.Json)DateJsonConverter.cs: JsonConverter that serializes Date to/from "yyyy-MM-dd" string formatTimeJsonConverter.cs: JsonConverter that serializes Time to/from "HH:mm:ss" string formatKiotaJsonSerializationContext.cs: AddedDefaultOptionsWithConvertersstatic property that provides pre-configured JsonSerializerOptions with converters registeredREADME.md: Added documentation with usage examples for the convertersTests
DateJsonConverterTests.cs: 8 comprehensive tests covering serialization, deserialization, round-trip, null handling, and error casesTimeJsonConverterTests.cs: 8 comprehensive tests covering serialization, deserialization, round-trip, null handling, and error casesTimeTests.cs: Removed JSON-specific tests from abstractions, keeping only type-specific testsUsage Examples:
Option 1: Using DefaultOptionsWithConverters
Option 2: Manually Registering Converters
Test Results:
Design Principles:
This approach maintains the key design principle that the abstractions project should be format-agnostic. All serialization-specific code is contained within the
Microsoft.Kiota.Serialization.Jsonpackage, allowing users who need System.Text.Json support to opt-in via the converters while keeping the core abstractions clean.Security Summary:
No security vulnerabilities were discovered or introduced by this change.
Original prompt
System.Text.Json.JsonSerializercannot deserializeDateandTimetypes using default converters #580✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.