Update/serialization deserialization#46
Merged
Merged
Conversation
…hance JsonSerializer to retrieve registered converters
… JsonConverters in BaseReflectionConverter
Contributor
Test Results 2 files 2 suites 3m 24s ⏱️ Results for commit 4e936e7. ♻️ This comment has been updated with latest results. |
…onverter handling and improving logging for blacklisted types
…ed logging and null handling
…od for clarity refactor: Remove commented-out code in GetSchema method for cleaner implementation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request enhances the serialization and deserialization system with improved custom JsonConverter integration, enhanced logging, and better API design. The changes add trace-level logging throughout the serialization pipeline, check for custom JsonConverters before using reflection-based converters, and expose converter management capabilities.
Key changes:
- Added
GetConverters()method to expose registered JSON converters - Integrated custom JsonConverter checks in serialization/deserialization paths before falling back to reflection converters
- Enhanced trace logging throughout the serialization pipeline with depth-based indentation
- Refined the
TryPopulateAPI to require a type parameter instead of optional fallback type
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| JsonSerializer.cs | Added GetConverters() method to retrieve registered converters |
| JsonSchema.cs | Added JsonSchemaExporterOptions configuration for null handling |
| Reflector.cs | Added JsonConverter checks before reflection converter usage, improved logging, removed obsolete documentation |
| Reflector.Populate.cs | Enhanced null checking and improved error message formatting |
| Reflector.Json.cs | Converted expression-bodied members to block bodies |
| Reflector.Error.cs | Added depth parameter to ToJson call |
| Reflector.CallMethod.cs | Added depth parameter to ToJson call |
| SerializedMember.cs | Added IsNull() helper method to check for null values |
| ExtensionsJson.cs | Changed ToJson return type to nullable, added depth parameter, updated default value behavior |
| GenericReflectionConverter.cs | Added depth parameter to ToJson call |
| BaseReflectionConverter.cs | Added filtering for ignored fields and properties |
| BaseReflectionConverter.Serialize.cs | Renamed parameter from type to fallbackType, improved log message formatting |
| BaseReflectionConverter.Populate.cs | Changed fallbackType parameter to required type parameter, simplified type resolution |
| ArrayReflectionConverter.cs | Enhanced trace logging for array serialization, added depth parameter |
| VersionJsonConverter.cs | Added JSON schema support by implementing IJsonSchemaConverter |
| BigIntegerJsonConverter.cs | Updated schema to use JsonSchema.Description constant |
| IReflectionConverter.cs | Changed TryPopulate signature to require type parameter |
| SerializedMemberSchemaTests.cs | Added null-forgiving operators for nullable ToJson return |
| SchemaSerializationValidationTests.cs | Added null-forgiving operators for nullable ToJson return |
| SerializePopulateTests.cs | Added success assertion and improved test output formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
This pull request improves the serialization logic in the reflection-based converters, with a focus on better integration with custom
JsonConverters and enhanced logging for debugging and traceability. The changes ensure that custom converters are used when available, provide more detailed trace logs, and expose the list of registered JSON converters for flexibility.Enhancements to serialization and logging:
JsonConverterfor the target type before proceeding with internal serialization. If a converter is found, it is used for serialization, and a detailed trace log is emitted. (BaseReflectionConverter.Serialize.cs)ArrayReflectionConverter.cs) [1] [2]ArrayReflectionConverter.cs)Improvements to JSON converter management:
GetConverters()method, allowing retrieval of all currently registeredJsonConverters. This supports dynamic inspection and management of converters. (JsonSerializer.cs)