Skip to content

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Fellmonkey
Copy link

What does this PR do?

This pull request introduces two major improvements to the .NET SDK generator:

  1. 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's System.Text.Json library. This includes:

    • Updated package dependencies in Package.csproj.twig
    • Migrated all serialization/deserialization logic in Client.cs.twig, Query.cs.twig, and Extensions.cs.twig
    • Updated model attributes from [JsonProperty] to [JsonPropertyName] in Model.cs.twig
    • Created new ObjectToInferredTypesConverter.cs.twig for proper object type handling
    • Replaced JsonConverter with JsonConverter<object> in ValueClassConverter.cs.twig
    • Updated error handling to use JsonDocument instead of JObject
  2. Adds initial test support for .NET 9.0 SDK - Extends platform compatibility by adding test infrastructure for the latest .NET version:

    • Created DotNet90Test.php test class for .NET 9.0 SDK validation
    • Added Tests90.csproj project file targeting .NET 9.0 framework

Test 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

@Fellmonkey
Copy link
Author

Also
update user-agent string interpolation to use string interpolation syntax
(appwrite/sdk-for-dotnet#63)
Generated code after changes:
image

@ChiragAgg5k ChiragAgg5k requested a review from Copilot June 10, 2025 07:41
Copy link
Contributor

@Copilot Copilot AI left a 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();

@ChiragAgg5k
Copy link
Member

ChiragAgg5k commented Jun 10, 2025

@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!

@Fellmonkey
Copy link
Author

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.

@ChiragAgg5k
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants