Skip to content

Conversation

@alexeyzimarev
Copy link
Member

@alexeyzimarev alexeyzimarev commented Nov 27, 2025

User description

Description

  • Add support for Microsoft DI container
  • Use HTTP client factory

Auto-created Ticket

#2319

PR Type

Enhancement


Description

  • Add Microsoft Dependency Injection container support via ServiceCollectionExtensions

  • Integrate HTTP client factory for managed HttpClient lifecycle

  • Extract shared request tests into reusable RequestTestsBase class

  • Reorganize test infrastructure and update namespace structure


Diagram Walkthrough

flowchart LR
  A["Microsoft DI Container"] -->|"AddRestClient"| B["ServiceCollectionExtensions"]
  B -->|"registers"| C["IRestClient"]
  B -->|"uses"| D["IHttpClientFactory"]
  D -->|"creates"| E["HttpClient"]
  E -->|"passed to"| F["RestClient"]
  G["RequestTestsBase"] -->|"shared tests"| H["RequestTests<br/>DependencyInjection"]
  G -->|"shared tests"| I["RequestTests<br/>Integrated"]
Loading

File Walkthrough

Relevant files
Enhancement
2 files
ServiceCollectionExtensions.cs
Microsoft DI extension methods for RestClient registration
+45/-0   
RestClient.cs
Expose HTTP configuration methods as internal                       
+2/-2     
Configuration changes
8 files
AssemblyInfo.cs
Grant internal visibility to DependencyInjection assembly
+3/-0     
Models.cs
Update namespace and make models public                                   
+2/-2     
WireMockTestServer.cs
Migrate to shared namespace and add TestRequest model       
+9/-4     
RestSharp.Tests.Integrated.csproj
Update using directives to reference shared namespace       
+2/-1     
RestSharp.Extensions.DependencyInjection.csproj
Create new DI extension library project file                         
+11/-0   
RestSharp.Tests.DependencyInjection.csproj
Create new DI tests project file                                                 
+12/-0   
RestSharp.csproj
Simplify compile dependencies using wildcard patterns       
+3/-42   
RestSharp.sln
Register new DI and DI test projects in solution                 
+65/-0   
Tests
3 files
RequestTestsBase.cs
Extract common request test cases into base class               
+117/-0 
RequestTests.cs
Implement DI-based request tests using shared base             
+21/-0   
RequestTests.cs
Refactor to inherit from RequestTestsBase                               
+6/-86   
Formatting
3 files
RedirectTests.cs
Remove redundant using statement for Server namespace       
+0/-2     
RequestFailureTests.cs
Remove redundant using statement for Server namespace       
+0/-2     
InterceptorTests.cs
Add blank line for formatting consistency                               
+1/-0     
Miscellaneous
1 files
PutTests.cs
Remove trailing TestRequest record definition                       
+0/-2     
Dependencies
1 files
Directory.Packages.props
Add Microsoft.Extensions.Http package dependency                 
+1/-0     

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 27, 2025

Deploying restsharp with  Cloudflare Pages  Cloudflare Pages

Latest commit: e6e7a0c
Status: ✅  Deploy successful!
Preview URL: https://e3894a39.restsharp.pages.dev
Branch Preview URL: https://depencency-injection.restsharp.pages.dev

View logs

@qodo-merge-for-open-source
Copy link
Contributor

qodo-merge-for-open-source bot commented Nov 27, 2025

PR Compliance Guide 🔍

(Compliance updated until commit 4227f68)

Below is a summary of compliance checks for this PR:

Security Compliance
Resource leak risk

Description: The HttpClientHandler is created without disposing it properly when
ConfigureHttpMessageHandler throws an exception, potentially causing resource leaks.
ServiceCollectionExtensions.cs [19-22]

Referred Code
    var handler = new HttpClientHandler();
    RestClient.ConfigureHttpMessageHandler(handler, options);
    return handler;
}
Ticket Compliance
🟢
🎫 #2319
🟢 Create RestSharp.Extensions.DependencyInjection package with ServiceCollectionExtensions
Provide AddRestClient() methods for various configuration scenarios
Integrate IHttpClientFactory for managed HttpClient creation and configuration
Extract common request test cases into RequestTestsBase for test reuse
Implement DI-specific test suite validating RestClient registration and execution
Codebase Duplication Compliance
🟢
No codebase code duplication found New Components Detected:
- ServiceCollectionExtensions
- extension
- RequestTestsBase
- TestClient
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null validation: The AddRestClient methods do not validate the options or baseUrl parameters for null
values before use.

Referred Code
public void AddRestClient(RestClientOptions options) {
    services
        .AddHttpClient(DefaultRestClient)
        .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options))
        .ConfigurePrimaryHttpMessageHandler(() => {
                var handler = new HttpClientHandler();
                RestClient.ConfigureHttpMessageHandler(handler, options);
                return handler;
            }
        );

    services.AddTransient<IRestClient>(sp => {
            var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(DefaultRestClient);
            return new RestClient(client, options);
        }
    );
}

/// <summary>
/// Adds a RestClient to the service collection with default options.
/// </summary>


 ... (clipped 9 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input validation missing: The baseUrl parameter in AddRestClient method is not validated before being passed to
RestClientOptions constructor.

Referred Code
    public void AddRestClient(string baseUrl) => services.AddRestClient(new RestClientOptions(baseUrl));
}

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 4dc910d
Security Compliance
🔴
Invalid extension syntax

Description: The extension keyword is invalid C# syntax and will cause compilation failure, preventing
the extension methods from being accessible.
ServiceCollectionExtensions.cs [8-8]

Referred Code
extension(IServiceCollection services) {
    /// <summary>
Internal API exposure

Description: Making ConfigureHttpClient and ConfigureHttpMessageHandler methods internal exposes
internal implementation details to external assemblies via InternalsVisibleTo, potentially
allowing unauthorized manipulation of HTTP client configuration.
RestClient.cs [227-232]

Referred Code
internal static void ConfigureHttpClient(HttpClient httpClient, RestClientOptions options) {
    if (options.Expect100Continue != null) httpClient.DefaultRequestHeaders.ExpectContinue = options.Expect100Continue;
}

// ReSharper disable once CognitiveComplexity
internal static void ConfigureHttpMessageHandler(HttpClientHandler handler, RestClientOptions options) {
Ticket Compliance
🎫 No ticket provided

[Generating...]

Codebase Duplication Compliance
🟢
No codebase code duplication found New Components Detected:
- ServiceCollectionExtensions
- extension
- RequestTestsBase
- TestClient
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null validation: The AddRestClient methods do not validate if the options or baseUrl parameters are null
before use, which could lead to runtime exceptions.

Referred Code
public void AddRestClient(RestClientOptions options) {
    services
        .AddHttpClient(DefaultRestClient)
        .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options))
        .ConfigurePrimaryHttpMessageHandler(() => {
                var handler = new HttpClientHandler();
                RestClient.ConfigureHttpMessageHandler(handler, options);
                return handler;
            }
        );

    services.AddTransient<IRestClient>(sp => {
            var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(DefaultRestClient);
            return new RestClient(client, options);
        }
    );
}

/// <summary>
/// Adds a RestClient to the service collection with default options.
/// </summary>


 ... (clipped 9 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input validation missing: The baseUrl parameter in AddRestClient(string baseUrl) is not validated for null, empty,
or malformed URL format before being passed to RestClientOptions.

Referred Code
    public void AddRestClient(string baseUrl) => services.AddRestClient(new RestClientOptions(baseUrl));
}

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-merge-for-open-source
Copy link
Contributor

qodo-merge-for-open-source bot commented Nov 27, 2025

PR Code Suggestions ✨

Latest suggestions up to e6e7a0c

CategorySuggestion                                                                                                                                    Impact
Incremental [*]
Fix invalid extension method syntax

Correct the invalid extension method syntax in ServiceCollectionExtensions.cs.
The extension(...) construct is not valid C# and should be replaced with
standard extension method declarations (using static methods with this on the
first parameter) to prevent compilation errors.

src/RestSharp.Extensions.DependencyInjection/ServiceCollectionExtensions.cs [7-15]

-extension(IServiceCollection services) {
+public static class ServiceCollectionExtensions {
     /// <summary>
     /// Adds a named RestClient to the service collection.
     /// </summary>
-    public void AddRestClient(
+    public static void AddRestClient(
+        this IServiceCollection services,
         string                  name,
         ConfigureRestClient?    configureRestClient    = null,
         ConfigureSerialization? configureSerialization = null
     ) {

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies that the extension(...) syntax used in the PR is invalid C# and will cause a compilation failure. Fixing this is critical for the new feature to work.

High
Remove redundant cast and null-forgiving operator

In InheritedCloneGenerator.cs, remove the redundant (bool) cast and the
unnecessary null-forgiving operator ! when calling GenerateMethod. The mutate
variable is already a non-nullable boolean.

gen/SourceGenerator/InheritedCloneGenerator.cs [45-54]

 var maybeMutate   = attributeData.NamedArguments.FirstOrDefault(arg => arg.Key == "Mutate").Value.Value;
 var mutate        = maybeMutate as bool? ?? false;
 ...
-var code = GenerateMethod(candidate, genericClassSymbol, (INamedTypeSymbol)baseType, methodName, (bool)mutate!);
+var code = GenerateMethod(candidate, genericClassSymbol, (INamedTypeSymbol)baseType, methodName, mutate);

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 4

__

Why: The suggestion correctly points out that the cast (bool) and the null-forgiving operator ! are redundant, as the variable mutate is already a non-nullable boolean. Removing them improves code clarity and style.

Low
Possible issue
Apply configuration dynamically for named clients

For named RestClient instances, resolve their configuration from the service
provider at runtime instead of capturing it at registration time. This ensures
the correct options are applied when the client is created.

src/RestSharp.Extensions.DependencyInjection/ServiceCollectionExtensions.cs [27-35]

-services
-    .AddHttpClient(name)
-    .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options))
-    .ConfigurePrimaryHttpMessageHandler(() => {
+if (name == Constants.DefaultRestClient) {
+    services
+        .AddHttpClient(name)
+        .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options))
+        .ConfigurePrimaryHttpMessageHandler(() => {
+                var handler = new HttpClientHandler();
+                RestClient.ConfigureHttpMessageHandler(handler, options);
+                return handler;
+            }
+        );
+}
+else {
+    services
+        .AddHttpClient(name)
+        .ConfigureHttpClient((sp, client) => {
+            var opts = sp.GetRequiredService<IOptionsMonitor<RestClientConfigOptions>>().Get(Constants.GetConfigName(name));
+            var tempOptions = new RestClientOptions();
+            opts.ConfigureRestClient?.Invoke(tempOptions);
+            RestClient.ConfigureHttpClient(client, tempOptions);
+        })
+        .ConfigurePrimaryHttpMessageHandler(sp => {
+            var opts = sp.GetRequiredService<IOptionsMonitor<RestClientConfigOptions>>().Get(Constants.GetConfigName(name));
+            var tempOptions = new RestClientOptions();
+            opts.ConfigureRestClient?.Invoke(tempOptions);
             var handler = new HttpClientHandler();
-            RestClient.ConfigureHttpMessageHandler(handler, options);
+            RestClient.ConfigureHttpMessageHandler(handler, tempOptions);
             return handler;
-        }
-    );
+        });
+}
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that for named clients, the options are not being resolved from the DI container at runtime, which defeats the purpose of using IOptions for configuration. Applying this change fixes a significant bug in the new dependency injection feature, ensuring that named clients are configured correctly.

Medium
  • More

Previous suggestions

✅ Suggestions up to commit 4dc910d
CategorySuggestion                                                                                                                                    Impact
General
Use a factory for client resolution
Suggestion Impact:The suggestion influenced the commit's approach to handling named clients. While the commit didn't implement the exact Func factory pattern suggested, it adopted a similar factory-based approach using IRestClientFactory (line 45) and added support for named clients with a default client registration (lines 47-52). The commit implements the core intent of supporting multiple named clients, though through a different architectural pattern (IRestClientFactory interface instead of Func delegate).

code diff:

+        public void AddRestClient(
+            string                  name,
+            ConfigureRestClient?    configureRestClient    = null,
+            ConfigureSerialization? configureSerialization = null
+        ) {
+            var options   = new RestClientOptions();
+            var configure = configureRestClient ?? (_ => { });
+            configure(options);
+
             services
-                .AddHttpClient(DefaultRestClient)
+                .AddHttpClient(name)
                 .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options))
                 .ConfigurePrimaryHttpMessageHandler(() => {
                         var handler = new HttpClientHandler();
@@ -22,24 +31,58 @@
                     }
                 );
 
-            services.AddTransient<IRestClient>(sp => {
-                    var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(DefaultRestClient);
-                    return new RestClient(client, options);
-                }
-            );
+            services.TryAddSingleton<IRestClientFactory, DefaultRestClientFactory>();
+
+            if (name == Constants.DefaultRestClient) {
+                services.AddTransient<IRestClient>(sp => {
+                        var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(name);
+                        return new RestClient(client, options);
+                    }
+                );
+            }
+            else {
+                services.Configure<RestClientConfigOptions>(
+                    Constants.GetConfigName(name),
+                    o => {
+                        o.ConfigureRestClient    = configureRestClient;
+                        o.ConfigureSerialization = configureSerialization;
+                    }
+                );
+            }

Register a factory (Func<string, IRestClient>) for IRestClient resolution to
allow consumers to dynamically resolve named clients, rather than registering a
single transient service.

src/RestSharp.Extensions.DependencyInjection/ServiceCollectionExtensions.cs [25-29]

-services.AddTransient<IRestClient>(sp => {
-        var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(DefaultRestClient);
-        return new RestClient(client, options);
-    }
-);
+services.AddTransient(sp => {
+    var factory = sp.GetRequiredService<IHttpClientFactory>();
+    return (string name) => new RestClient(factory.CreateClient(name), options);
+});
+services.AddTransient<IRestClient>(sp => sp.GetRequiredService<Func<string, IRestClient>>()(DefaultRestClient));

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion provides a robust and correct solution using a factory pattern (Func<string, IRestClient>) to handle the registration and resolution of multiple named RestClient instances, which is a significant improvement over the PR's implementation.

Medium
Allow registering multiple named clients
Suggestion Impact:The suggestion's core idea of accepting a name parameter was implemented. The commit added a name parameter to AddRestClient (line 22) and uses it to register the HttpClient (line 32) and create the client (line 49). However, the implementation differs significantly: it uses a more sophisticated approach with a factory pattern, configuration options, and special handling for the default client vs named clients (lines 47-62). The commit also added multiple overloads and changed the signature to use configuration delegates instead of directly passing RestClientOptions.

code diff:

+        public void AddRestClient(
+            string                  name,
+            ConfigureRestClient?    configureRestClient    = null,
+            ConfigureSerialization? configureSerialization = null
+        ) {
+            var options   = new RestClientOptions();
+            var configure = configureRestClient ?? (_ => { });
+            configure(options);
+
             services
-                .AddHttpClient(DefaultRestClient)
+                .AddHttpClient(name)
                 .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options))
                 .ConfigurePrimaryHttpMessageHandler(() => {
                         var handler = new HttpClientHandler();
@@ -22,24 +31,58 @@
                     }
                 );
 
-            services.AddTransient<IRestClient>(sp => {
-                    var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(DefaultRestClient);
-                    return new RestClient(client, options);
-                }
-            );
+            services.TryAddSingleton<IRestClientFactory, DefaultRestClientFactory>();
+
+            if (name == Constants.DefaultRestClient) {
+                services.AddTransient<IRestClient>(sp => {
+                        var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(name);
+                        return new RestClient(client, options);
+                    }
+                );
+            }
+            else {
+                services.Configure<RestClientConfigOptions>(
+                    Constants.GetConfigName(name),
+                    o => {
+                        o.ConfigureRestClient    = configureRestClient;
+                        o.ConfigureSerialization = configureSerialization;
+                    }
+                );
+            }

Modify AddRestClient to accept a name parameter to support registering multiple
named RestClient instances, instead of using a hardcoded name.

src/RestSharp.Extensions.DependencyInjection/ServiceCollectionExtensions.cs [8-30]

 extension(IServiceCollection services) {
     /// <summary>
     /// Adds a RestClient to the service collection.
     /// </summary>
+    /// <param name="name">The name of the client.</param>
     /// <param name="options">The configuration options for the RestClient.</param>
     [PublicAPI]
-    public void AddRestClient(RestClientOptions options) {
+    public void AddRestClient(string name, RestClientOptions options) {
         services
-            .AddHttpClient(DefaultRestClient)
+            .AddHttpClient(name)
             .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options))
             .ConfigurePrimaryHttpMessageHandler(() => {
                     var handler = new HttpClientHandler();
                     RestClient.ConfigureHttpMessageHandler(handler, options);
                     return handler;
                 }
             );
 
         services.AddTransient<IRestClient>(sp => {
-                var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(DefaultRestClient);
+                var client = sp.GetRequiredService<IHttpClientFactory>().CreateClient(name);
                 return new RestClient(client, options);
             }
         );
     }

[Suggestion processed]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a limitation with the hardcoded client name, but the proposed implementation is flawed as it would overwrite the IRestClient registration, making it impossible to resolve different named clients.

Low

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

Test Results

   42 files     42 suites   18m 20s ⏱️
  477 tests   477 ✅ 0 💤 0 ❌
3 334 runs  3 334 ✅ 0 💤 0 ❌

Results for commit e6e7a0c.

♻️ This comment has been updated with latest results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants