Skip to content

Fix #2402: defer default serializer initialization - #2412

Closed
segor wants to merge 1 commit into
restsharp:devfrom
segor:fix-2402-lazy-serializers
Closed

segor wants to merge 1 commit into
restsharp:devfrom
segor:fix-2402-lazy-serializers

Conversation

@segor

@segor segor commented Sep 25, 2026

Copy link
Copy Markdown

Description

Fixes #2402.

Register default serializers without constructing them, allowing a custom JSON serializer such as Newtonsoft.Json to replace the default before System.Text.Json is loaded.

Share content-type matching predicates between registration and the built-in serializers to avoid duplicated rules. Public APIs and serializer instance lifetimes remain unchanged.

Uses the deferred-registration approach proposed in #2403, with shared matching rules and automated regression coverage.

Validation:

  • Release build passed for all six current library targets.
  • Five isolated net48 AppDomain tests verify that only the required JSON assemblies load.
  • Four regression scenarios failed against the original implementation.
  • Added tests for registration metadata consistency and fresh serializer instances.

No user-facing documentation changes are needed because existing configuration APIs remain unchanged.

Purpose

This pull request is a:

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Defer default serializer construction to avoid unnecessary JSON loading

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Register built-in serializers lazily so replacing JSON avoids loading System.Text.Json.
• Centralize JSON and XML content-type predicates across registration and serializer
 implementations.
• Add isolated assembly-loading, metadata-consistency, and fresh-instance regression coverage.
Diagram

sequenceDiagram
    participant C as RestClient
    participant SC as Serializer Config
    participant R as Serializer Registry
    participant CJ as Custom JSON
    participant DF as Default Factory
    participant JA as JSON Assembly
    C->>SC: Initialize serializers
    SC->>R: Store deferred defaults
    Note over DF,JA: Not loaded during registration
    alt Custom JSON configured
        C->>SC: Configure custom JSON
        SC->>CJ: Inspect metadata
        SC->>R: Replace JSON record
        R->>CJ: Create selected serializer
    else Default JSON requested
        R->>DF: Invoke factory
        DF->>JA: Load dependency
        DF-->>R: Return fresh serializer
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Cache serializers with Lazy
  • ➕ Naturally delays dependency loading until first use
  • ➕ Centralizes deferred initialization behavior
  • ➖ Would reuse serializer instances and change existing lifetime semantics
  • ➖ Still requires metadata without constructing the serializer
2. Add a static serializer metadata contract
  • ➕ Keeps metadata colocated with each serializer type
  • ➕ Reduces manual descriptor construction in registration
  • ➖ Requires a broader serializer contract and compatibility change
  • ➖ Adds complexity for custom serializers and older target frameworks

Recommendation: Keep the PR's deferred descriptor approach. It fixes premature dependency loading with a narrow internal change, preserves fresh serializer instances and existing public configuration APIs, and avoids the compatibility costs of a new metadata contract. Shared ContentType predicates limit the main risk of manually registered metadata drifting from built-in implementations.

Files changed (6) +164 / -7

Bug fix (1) +15 / -1
SerializerConfig.csRegister built-in serializers without constructing them +15/-1

Register built-in serializers without constructing them

• Creates default JSON and XML SerializerRecord entries directly from static metadata and deferred factories. Custom configuration can therefore replace System.Text.Json before its serializer or dependency is loaded, while each lookup still returns a fresh instance.

src/RestSharp/Serializers/SerializerConfig.cs

Refactor (3) +8 / -5
ContentType.csCentralize JSON and XML matching predicates +4/-1

Centralize JSON and XML matching predicates

• Adds shared internal content-type predicates for JSON and XML. Registration descriptors and built-in serializers can now use identical matching rules.

src/RestSharp/ContentType.cs

SystemTextJsonSerializer.csReuse the shared JSON content-type predicate +2/-2

Reuse the shared JSON content-type predicate

• Replaces the serializer-local JSON suffix predicate with the centralized ContentType.SupportsJson delegate, keeping runtime metadata aligned with deferred registration.

src/RestSharp/Serializers/Json/SystemTextJsonSerializer.cs

XmlRestSerializer.csReuse the shared XML content-type predicate +2/-2

Reuse the shared XML content-type predicate

• Replaces the serializer-local XML suffix predicate with ContentType.SupportsXml so deferred registration and serializer metadata share the same delegate.

src/RestSharp/Serializers/Xml/XmlRestSerializer.cs

Tests (2) +141 / -1
SerializerAssemblyLoadingTests.csVerify JSON dependencies load only when selected +125/-0

Verify JSON dependencies load only when selected

• Adds net48 AppDomain-isolated tests covering defaults, XML-only use, default JSON, Newtonsoft replacement, and Newtonsoft-only configuration. Round-trip checks confirm functionality while loaded-assembly assertions detect premature System.Text.Json or Newtonsoft.Json loading.

test/RestSharp.Tests.Serializers.Json/SerializerAssemblyLoadingTests.cs

RestClientTests.csVerify default metadata and serializer lifetimes +16/-1

Verify default metadata and serializer lifetimes

• Adds JSON and XML regression cases confirming registration metadata matches constructed serializers, including delegate identity. It also verifies repeated lookups continue producing fresh serializer instances.

test/RestSharp.Tests/RestClientTests.cs

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@sonarqubecloud

Copy link
Copy Markdown

@segor segor closed this Sep 25, 2026
@segor
segor deleted the fix-2402-lazy-serializers branch September 25, 2026 17:47
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.

RestClient configured with Newtonsoft.Json serializer still instantiates System.Text.Json

1 participant