-
Notifications
You must be signed in to change notification settings - Fork 181
feat: migrate .NET templates to System.Text.Json + add .NET 9.0 tests #1085
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…ization/deserialization
Also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request migrates the .NET SDK from Newtonsoft.Json to System.Text.Json while also adding support for .NET 9.0 tests. Key changes include the conversion of JSON serialization/deserialization logic and model attributes, updates to project dependencies and target frameworks, and the addition of new test files for .NET 9.0.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/languages/dotnet/Tests90.csproj | New project file targeting .NET 9.0 |
tests/DotNet90Test.php | New test class to validate the .NET 9.0 SDK functionality |
templates/dotnet/README.md.twig | Updated documentation to reference System.Text.Json and the new attribute naming |
templates/dotnet/Package/Query.cs.twig | Updated Query implementation to use System.Text.Json for serialization/deserialization |
templates/dotnet/Package/Package.csproj.twig | Updated target frameworks and package references (switching from Newtonsoft.Json to System.Text.Json) |
templates/dotnet/Package/Models/Model.cs.twig | Replaced JsonProperty with JsonPropertyName and adapted deserialization logic |
templates/dotnet/Package/Models/InputFile.cs.twig | Set default initializers for properties; note the change for Data initialization |
templates/dotnet/Package/Extensions/Extensions.cs.twig | Updated extension methods to use System.Text.Json serialization |
templates/dotnet/Package/Converters/ValueClassConverter.cs.twig | Migrated ValueClassConverter to use the new JSON APIs |
templates/dotnet/Package/Converters/ObjectToInferredTypesConverter.cs.twig | Added a new converter for inferring object types with System.Text.Json |
templates/dotnet/Package/Client.cs.twig | Updated client logic including error handling, file upload validations, and JSON serialization |
Comments suppressed due to low confidence (1)
templates/dotnet/Package/Models/InputFile.cs.twig:12
- Initializing Data with a new object() may lead to runtime casting issues when InputFile.Data is expected to be a Stream or byte array. Consider making Data nullable or initializing it to null instead.
public object Data { get; set; } = new object();
@Fellmonkey Thank you for submitting this PR! This feature has been highly requested by our community, so we really appreciate you taking the initiative to implement it. I noticed that the .NET SDK build is currently failing in the test suite. Could you please take a look at the failing tests and address any issues? Once those are resolved, we'll be happy to move forward with the review process. Thanks again for your contribution! |
Add missing ObjectToInferredTypesConverter.cs template configuration Previously forgot to include the ObjectToInferredTypesConverter.cs template in the files array, which caused tests to fail due to missing converter dependency. |
@Fellmonkey Great work! 🔥 All tests are passing. We'll just need a review from one of our more experienced .NET engineers before we can merge this. |
What does this PR do?
This pull request introduces two major improvements to the .NET SDK generator:
Refactors from Newtonsoft.Json to System.Text.Json for serialization/deserialization - A comprehensive migration that modernizes the .NET SDK by replacing the legacy
Newtonsoft.Json
library with Microsoft'sSystem.Text.Json library
. This includes:Package.csproj.twig
Client.cs.twig
,Query.cs.twig
, andExtensions.cs.twig
[JsonProperty]
to[JsonPropertyName]
inModel.cs.twig
ObjectToInferredTypesConverter.cs.twig
for proper object type handlingJsonConverter
withJsonConverter<object>
inValueClassConverter.cs.twig
JsonDocument
instead ofJObject
Adds initial test support for .NET 9.0 SDK - Extends platform compatibility by adding test infrastructure for the latest .NET version:
DotNet90Test.php
test class for .NET 9.0 SDK validationTests90.csproj
project file targeting .NET 9.0 frameworkTest Plan
Regenerate .NET SDKs using the updated templates and verify that existing client code continues to work without changes
Related PRs and Issues
(appwrite/sdk-for-dotnet#48)
Have you read the Contributing Guidelines on issues?
Yes