Add DocumentJsonConverter for System.Text.Json serialization support#4347
Add DocumentJsonConverter for System.Text.Json serialization support#4347AlexDaines wants to merge 5 commits intodevelopmentfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes System.Text.Json serialization/deserialization for Amazon.Runtime.Documents.Document by adding a dedicated STJ converter so Document is no longer treated as an IEnumerable collection during serialization, and aligns Document.FromObject numeric parsing behavior with existing runtime unmarshalling logic.
Changes:
- Added an internal
DocumentJsonConverterand applied it toDocumentvia[JsonConverter]. - Updated
Document.FromObjectto preferTryGetInt32beforeTryGetInt64. - Added unit tests covering primitive/collection serialization, round-trips, and regressions; added a patch DevConfig entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sdk/src/Core/Amazon.Runtime/Documents/DocumentJsonConverter.cs | Implements STJ converter to serialize/deserialize Document by its DocumentType rather than via IEnumerable. |
| sdk/src/Core/Amazon.Runtime/Documents/Document.cs | Applies the converter attribute and updates FromObject number parsing to try int before long. |
| sdk/test/UnitTests/Custom/Runtime/Documents/DocumentJsonConverterTests.cs | Adds coverage for the new converter, including regressions and customer scenarios. |
| generator/.DevConfigs/180bd698-bd6a-4799-a23e-db90f89660d6.json | Declares a patch-level core changelog entry for the fix. |
sdk/test/UnitTests/Custom/Runtime/Documents/DocumentJsonConverterTests.cs
Outdated
Show resolved
Hide resolved
normj
left a comment
There was a problem hiding this comment.
Very cool PR. This will help make working with JSON and Documents.
| [TestMethod] | ||
| [TestCategory("UnitTest")] | ||
| [TestCategory("Runtime")] | ||
| public void SerializeFilterAttributeScenario() |
There was a problem hiding this comment.
This test seems to be a duplicate of SerializeObjectContainingDocument (that also mentions #3694).
Also, I don't see any tests referencing #3837, is that because it's covered by another test?
Description
Add
DocumentJsonConverterto fixSystem.Text.Jsonserialization ofDocumenttypes.DocumentimplementsIEnumerableinterfaces which causes STJ to treat it as a collection, throwingInvalidDocumentTypeConversionExceptionfor non-list/non-dictionary types.Also fixes
Document.FromObjectnumber parsing to tryTryGetInt32beforeTryGetInt64, consistent withDocumentUnmarshaller.Motivation and Context
Fixes #3694, #3837, #4078.
Testing
dotnet test --filter "FullyQualifiedName~Document"→ 26 passed, 0 failed.Breaking Changes Assessment
No breaking changes.
DocumentJsonConverterisinternal sealed. The[JsonConverter]attribute changes STJ behavior from broken (throws) to correct. Customer workarounds (custom converters inJsonSerializerOptions) take precedence per STJ resolution order.Types of changes
Checklist
License