Skip to content

Commit 991130b

Browse files
alexeyzimarevclaude
alexeyzimarev
and
claude
authored
Fix default parameter merging bugs, add MergedParameters to RestResponse (#2349)
* Fix default parameters missing from RestResponse.Request.Parameters (#2282) Merge default parameters into RestRequest.Parameters early in ExecuteRequestAsync so they are visible via response.Request.Parameters. Remove the now-redundant separate merges in RequestContent, BuildUriExtensions, RequestHeaders, and OAuth1Authenticator. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add other files * Fix default parameter merging bugs introduced by #2282 Revert per-site merging to restore original design and fix three bugs: 1. Multi-value dedup — same-name defaults (AllowMultipleDefaultParametersWithSameName) were silently dropped 2. Public API breakage — BuildUriString/GetRequestQuery didn't include defaults when called outside ExecuteAsync 3. Request mutation — stale defaults persisted on reused requests when DefaultParameters changed Add MergedParameters property on RestResponse to satisfy the original #2282 requirement of making default parameters visible after execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix CI: run dotnet test on individual test projects The addition of RestSharp.slnx alongside RestSharp.sln causes MSB1011 ("more than one project or solution file") when running bare dotnet test. Run each test project explicitly instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Make MergedParameters non-null with internal setter - Initialize to empty RequestParameters() so consumers never need null checks - Restrict setter to internal to prevent external mutation of response state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Document MergedParameters property in response docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f2b1fd1 commit 991130b

8 files changed

Lines changed: 192 additions & 3 deletions

File tree

‎.github/workflows/pull-request.yml‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ jobs:
3737
9.0.x
3838
10.0.x
3939
- name: Run tests
40-
run: dotnet test -c Debug -f ${{ matrix.dotnet }}
40+
run: |
41+
dotnet test test/RestSharp.Tests -c Debug -f ${{ matrix.dotnet }}
42+
dotnet test test/RestSharp.Tests.Integrated -c Debug -f ${{ matrix.dotnet }}
43+
dotnet test test/RestSharp.Tests.Serializers.Json -c Debug -f ${{ matrix.dotnet }}
44+
dotnet test test/RestSharp.Tests.Serializers.Xml -c Debug -f ${{ matrix.dotnet }}
45+
dotnet test test/RestSharp.Tests.Serializers.Csv -c Debug -f ${{ matrix.dotnet }}
46+
dotnet test test/RestSharp.Tests.DependencyInjection -c Debug -f ${{ matrix.dotnet }}
4147
- name: Upload Test Results
4248
if: always()
4349
uses: actions/upload-artifact@v6
@@ -65,7 +71,13 @@ jobs:
6571
9.0.x
6672
10.0.x
6773
- name: Run tests
68-
run: dotnet test -f ${{ matrix.dotnet }}
74+
run: |
75+
dotnet test test/RestSharp.Tests -f ${{ matrix.dotnet }}
76+
dotnet test test/RestSharp.Tests.Integrated -f ${{ matrix.dotnet }}
77+
dotnet test test/RestSharp.Tests.Serializers.Json -f ${{ matrix.dotnet }}
78+
dotnet test test/RestSharp.Tests.Serializers.Xml -f ${{ matrix.dotnet }}
79+
dotnet test test/RestSharp.Tests.Serializers.Csv -f ${{ matrix.dotnet }}
80+
dotnet test test/RestSharp.Tests.DependencyInjection -f ${{ matrix.dotnet }}
6981
- name: Upload Test Results
7082
if: always()
7183
uses: actions/upload-artifact@v6

‎CLAUDE.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
## MCP Servers Available
2+
- mem0: Use this AI memory for storing and retrieving long-term context as well as short-term context

‎RestSharp.slnx‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<Solution>
2+
<Configurations>
3+
<BuildType Name="Debug" />
4+
<BuildType Name="Debug.Appveyor" />
5+
<BuildType Name="Release" />
6+
<Platform Name="Any CPU" />
7+
<Platform Name="ARM" />
8+
<Platform Name="Mixed Platforms" />
9+
<Platform Name="x64" />
10+
<Platform Name="x86" />
11+
</Configurations>
12+
<Folder Name="/Perf/">
13+
<Project Path="benchmarks/RestSharp.Benchmarks/RestSharp.Benchmarks.csproj">
14+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
15+
</Project>
16+
</Folder>
17+
<Folder Name="/Serializers/">
18+
<Project Path="src/RestSharp.Serializers.CsvHelper/RestSharp.Serializers.CsvHelper.csproj">
19+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
20+
</Project>
21+
<Project Path="src/RestSharp.Serializers.NewtonsoftJson/RestSharp.Serializers.NewtonsoftJson.csproj">
22+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
23+
</Project>
24+
<Project Path="src/RestSharp.Serializers.Xml/RestSharp.Serializers.Xml.csproj">
25+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
26+
</Project>
27+
</Folder>
28+
<Folder Name="/SourceGen/">
29+
<Project Path="gen/SourceGenerator/SourceGenerator.csproj">
30+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
31+
</Project>
32+
</Folder>
33+
<Folder Name="/Tests/">
34+
<Project Path="test/RestSharp.InteractiveTests/RestSharp.InteractiveTests.csproj">
35+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
36+
</Project>
37+
<Project Path="test/RestSharp.Tests.DependencyInjection/RestSharp.Tests.DependencyInjection.csproj">
38+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
39+
</Project>
40+
<Project Path="test/RestSharp.Tests.Integrated/RestSharp.Tests.Integrated.csproj">
41+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
42+
</Project>
43+
<Project Path="test/RestSharp.Tests.Serializers.Csv/RestSharp.Tests.Serializers.Csv.csproj">
44+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
45+
</Project>
46+
<Project Path="test/RestSharp.Tests.Serializers.Json/RestSharp.Tests.Serializers.Json.csproj">
47+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
48+
</Project>
49+
<Project Path="test/RestSharp.Tests.Serializers.Xml/RestSharp.Tests.Serializers.Xml.csproj">
50+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
51+
</Project>
52+
<Project Path="test/RestSharp.Tests.Shared/RestSharp.Tests.Shared.csproj">
53+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
54+
</Project>
55+
<Project Path="test/RestSharp.Tests/RestSharp.Tests.csproj">
56+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
57+
</Project>
58+
</Folder>
59+
<Project Path="src/RestSharp.Extensions.DependencyInjection/RestSharp.Extensions.DependencyInjection.csproj">
60+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
61+
</Project>
62+
<Project Path="src/RestSharp/RestSharp.csproj">
63+
<BuildType Solution="Debug.Appveyor|*" Project="Debug" />
64+
</Project>
65+
</Solution>

‎docs/docs/usage/response.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,19 @@ Response object contains the following properties:
2828
| `ErrorException` | `Exception?` | Exception thrown when executing the request, if any. |
2929
| `Version` | `Version?` | HTTP protocol version of the request. |
3030
| `RootElement` | `string?` | Root element of the serialized response content, only works if deserializer supports it. |
31+
| `MergedParameters` | `ParametersCollection` | Combined view of request parameters and client default parameters at execution time. |
32+
33+
### Merged parameters
34+
35+
The `MergedParameters` property provides a combined view of the request's own parameters and the client's [default parameters](request.md#request-headers) as they were at execution time. This is useful for logging or debugging the full set of parameters that were applied to a request, since `Request.Parameters` only contains the parameters added directly to the request.
36+
37+
```csharp
38+
var response = await client.ExecuteAsync(request);
39+
40+
foreach (var param in response.MergedParameters) {
41+
Console.WriteLine($"{param.Name} = {param.Value} ({param.Type})");
42+
}
43+
```
3144

3245
In addition, `RestResponse<T>` has one additional property:
3346

‎src/RestSharp/Response/RestResponseBase.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ protected RestResponseBase(RestRequest request) {
132132
/// </summary>
133133
public Version? Version { get; set; }
134134

135+
/// <summary>
136+
/// Combined view of request parameters and client default parameters as they were at execution time.
137+
/// Use this to inspect the full set of parameters that were applied to the request.
138+
/// </summary>
139+
public ParametersCollection MergedParameters { get; internal set; } = new RequestParameters();
140+
135141
/// <summary>
136142
/// Root element of the serialized response content, only works if deserializer supports it
137143
/// </summary>

‎src/RestSharp/RestClient.Async.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public async Task<RestResponse> ExecuteAsync(RestRequest request, CancellationTo
3636
)
3737
.ConfigureAwait(false)
3838
: GetErrorResponse(request, internalResponse.Exception, internalResponse.TimeoutToken);
39+
response.MergedParameters = new RequestParameters(request.Parameters.Union(DefaultParameters));
3940
await OnAfterRequest(response, cancellationToken).ConfigureAwait(false);
4041

4142
return Options.ThrowOnAnyError ? response.ThrowIfError() : response;

‎test/RestSharp.Tests.Integrated/DefaultParameterTests.cs‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
namespace RestSharp.Tests.Integrated;
55

66
public sealed class DefaultParameterTests(WireMockTestServer server) : IClassFixture<WireMockTestServer> {
7-
readonly RequestBodyCapturer _capturer = server.ConfigureBodyCapturer(Method.Get, false);
7+
readonly RequestBodyCapturer _capturer = server.ConfigureBodyCapturer(Method.Get, false);
8+
readonly RequestBodyCapturer _capturerOnPath = server.ConfigureBodyCapturer(Method.Get);
89

910
[Fact]
1011
public async Task Should_add_default_and_request_query_get_parameters() {
@@ -48,4 +49,74 @@ public async Task Should_not_encode_pipe_character_when_encode_is_false() {
4849
var query = _capturer.RawUrl.Split('?')[1];
4950
query.Should().Contain("ids=in:001|116");
5051
}
52+
53+
[Fact]
54+
public async Task Should_include_multiple_default_query_params_with_same_name() {
55+
using var client = new RestClient(
56+
new RestClientOptions(server.Url!) { AllowMultipleDefaultParametersWithSameName = true }
57+
);
58+
client.AddDefaultParameter("filter", "active", ParameterType.QueryString);
59+
client.AddDefaultParameter("filter", "verified", ParameterType.QueryString);
60+
61+
var request = new RestRequest("capture");
62+
await client.GetAsync(request);
63+
64+
var query = _capturerOnPath.Url!.Query;
65+
query.Should().Contain("filter=active");
66+
query.Should().Contain("filter=verified");
67+
}
68+
69+
[Fact]
70+
public async Task Should_include_default_query_params_in_BuildUriString_without_executing() {
71+
using var client = new RestClient(server.Url!);
72+
client.AddDefaultParameter("foo", "bar", ParameterType.QueryString);
73+
74+
var request = new RestRequest("resource");
75+
var uri = client.BuildUriString(request);
76+
77+
uri.Should().Contain("foo=bar");
78+
}
79+
80+
[Fact]
81+
public async Task Should_not_permanently_mutate_request_parameters_after_execute() {
82+
using var client = new RestClient(server.Url!);
83+
client.AddDefaultParameter("default_key", "default_val", ParameterType.QueryString);
84+
85+
var request = new RestRequest("capture");
86+
var paramsBefore = request.Parameters.Count;
87+
88+
await client.GetAsync(request);
89+
90+
// Request parameters should not have been mutated by the execution.
91+
request.Parameters.Count.Should().Be(paramsBefore);
92+
93+
// Now replace the default parameter with a different value.
94+
client.DefaultParameters.ReplaceParameter(new QueryParameter("default_key", "updated_val"));
95+
96+
await client.GetAsync(request);
97+
98+
// The second execution should use the updated default value, not the stale one.
99+
var query = _capturerOnPath.Url!.Query;
100+
query.Should().Contain("default_key=updated_val");
101+
query.Should().NotContain("default_key=default_val");
102+
}
103+
104+
[Fact]
105+
public async Task Should_include_default_params_in_merged_parameters_on_response() {
106+
using var client = new RestClient(server.Url!);
107+
client.AddDefaultParameter("default_key", "default_val", ParameterType.QueryString);
108+
109+
var request = new RestRequest("capture").AddQueryParameter("req_key", "req_val");
110+
var response = await client.ExecuteAsync(request);
111+
112+
var defaultParam = response.MergedParameters
113+
.FirstOrDefault(p => p.Name == "default_key" && p.Type == ParameterType.QueryString);
114+
defaultParam.Should().NotBeNull();
115+
defaultParam!.Value.Should().Be("default_val");
116+
117+
var requestParam = response.MergedParameters
118+
.FirstOrDefault(p => p.Name == "req_key" && p.Type == ParameterType.QueryString);
119+
requestParam.Should().NotBeNull();
120+
requestParam!.Value.Should().Be("req_val");
121+
}
51122
}

‎test/RestSharp.Tests.Integrated/HttpHeadersTests.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,25 @@ public async Task Should_sent_custom_UserAgent() {
6868
response.GetHeaderValue("Server").Should().Be("Kestrel");
6969
}
7070

71+
[Fact]
72+
public async Task Default_headers_should_appear_in_response_merged_parameters() {
73+
const string headerName = "X-Custom-Default";
74+
const string headerValue = "DefaultValue123";
75+
76+
_client.AddDefaultHeader(headerName, headerValue);
77+
78+
var request = new RestRequest("/headers");
79+
var response = await _client.ExecuteAsync(request);
80+
81+
response.StatusCode.Should().Be(HttpStatusCode.OK);
82+
83+
var param = response.MergedParameters
84+
.FirstOrDefault(p => p.Name == headerName && p.Type == ParameterType.HttpHeader);
85+
86+
param.Should().NotBeNull();
87+
param!.Value.Should().Be(headerValue);
88+
}
89+
7190
static void CheckHeader(RestResponse<TestServerResponse[]> response, Header header) {
7291
var h = FindHeader(response, header.Name);
7392
h.Should().NotBeNull();

0 commit comments

Comments
 (0)