Skip to content

Commit ff138ea

Browse files
hamzamahmoodhamzamahmood
authored
fix(request-parameter): deduplicate parameters using ConcurrentDictionary (#129)
1 parent 165b139 commit ff138ea

File tree

3 files changed

+228
-25
lines changed

3 files changed

+228
-25
lines changed

‎.github/workflows/test.yml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ jobs:
3838
if: ${{ matrix.os == 'ubuntu-22.04' && matrix.dotnet == '7.0.x' }}
3939
run: dotnet test APIMatic.Core.Test/APIMatic.Core.Test.csproj -p:CollectCoverage=true -p:CoverletOutputFormat=lcov
4040

41-
- name: Upload coverage report
42-
if: ${{ matrix.os == 'ubuntu-22.04' && matrix.dotnet == '7.0.x' && github.actor != 'dependabot[bot]' }}
43-
uses: paambaati/[email protected]
44-
env:
45-
CC_TEST_REPORTER_ID: ${{ secrets.CODE_CLIMATE_KEY }}
46-
with:
47-
coverageLocations: |
48-
${{github.workspace}}/APIMatic.Core.Test/coverage.info:lcov
41+
# - name: Upload coverage report
42+
# if: ${{ matrix.os == 'ubuntu-22.04' && matrix.dotnet == '7.0.x' && github.actor != 'dependabot[bot]' }}
43+
# uses: paambaati/[email protected]
44+
# env:
45+
# CC_TEST_REPORTER_ID: ${{ secrets.CODE_CLIMATE_KEY }}
46+
# with:
47+
# coverageLocations: |
48+
# ${{github.workspace}}/APIMatic.Core.Test/coverage.info:lcov

‎APIMatic.Core.Test/Request/ParameterTests.cs‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using APIMatic.Core.Request;
4+
using APIMatic.Core.Request.Parameters;
5+
using NUnit.Framework;
6+
7+
namespace APIMatic.Core.Test.Request
8+
{
9+
[TestFixture]
10+
public class ParameterTests : TestBase
11+
{
12+
const string ServerUrl = "https://parameter.api.server.com";
13+
14+
private static readonly Dictionary<string, object> QueryParams = new Dictionary<string, object>
15+
{
16+
["field"] = "query_value",
17+
["field1"] = "query_value1",
18+
["field2"] = "query_value2",
19+
["field3"] = "query_value3",
20+
};
21+
22+
private static readonly Dictionary<string, object> AdditionalQueryParams = new Dictionary<string, object>
23+
{
24+
["field"] = "additional_query_value",
25+
["field1"] = "additional_query_value1",
26+
["field2"] = "additional_query_value2",
27+
["field3"] = "additional_query_value3",
28+
};
29+
30+
[Test]
31+
public void AdditionalForms_WhenKeyIsNull_ShouldAssignInnerFieldsCorrectly()
32+
{
33+
// Arrange
34+
var parameters = new Parameter.Builder();
35+
var fieldParameters = new Dictionary<string, object> { ["field"] = "field", };
36+
37+
parameters
38+
.AdditionalForms(additionalForms => additionalForms.Setup(fieldParameters));
39+
40+
var requestBuilder = new RequestBuilder(LazyGlobalConfiguration.Value, ServerUrl);
41+
42+
// Act
43+
parameters.Validate().Apply(requestBuilder);
44+
45+
// Assert
46+
var formParam = requestBuilder.formParameters.FirstOrDefault(p => p.Key == "field");
47+
48+
Assert.Multiple(() =>
49+
{
50+
Assert.That(formParam, Is.Not.Null, "Expected form parameter with key 'field'");
51+
Assert.That(formParam.Value, Is.EqualTo("field"), "Expected value mismatch for 'field'");
52+
});
53+
}
54+
55+
[Test]
56+
public void DuplicateHeaderKeys_Should_UseLastOneInRequestBuilder()
57+
{
58+
// Arrange
59+
var parameters = new Parameter.Builder();
60+
61+
parameters
62+
.Header(h => h.Setup("Authorization", "Basic OLD1").Required())
63+
.Header(h => h.Setup("Authorization", "Basic OLD2").Required())
64+
.Header(h => h.Setup("Authorization", "Basic OLD3").Required())
65+
.Header(h => h.Setup("Authorization", "Basic OLD4").Required())
66+
.Header(h => h.Setup("Authorization", "Basic OLD5").Required())
67+
.Header(h => h.Setup("Authorization", "Basic OLD6").Required())
68+
.Header(h => h.Setup("Authorization", "Basic NEW").Required());
69+
70+
var requestBuilder = new RequestBuilder(LazyGlobalConfiguration.Value, ServerUrl);
71+
72+
// Act
73+
parameters.Validate().Apply(requestBuilder);
74+
75+
// Assert
76+
Assert.That(requestBuilder.headersParameters.TryGetValue("Authorization", out var value), Is.True);
77+
Assert.That(value, Is.EqualTo("Basic NEW"));
78+
}
79+
80+
[Test]
81+
public void DuplicateQueryKeys_Should_UseLastOneInRequestBuilder()
82+
{
83+
// Arrange
84+
var parameters = new Parameter.Builder();
85+
86+
parameters
87+
.Query(q => q.Setup("status", "pending").Required())
88+
.Query(q => q.Setup("status", "approved").Required());
89+
90+
var requestBuilder = new RequestBuilder(LazyGlobalConfiguration.Value, ServerUrl);
91+
92+
// Act
93+
parameters.Validate().Apply(requestBuilder);
94+
95+
// Assert
96+
Assert.That(requestBuilder.queryParameters.TryGetValue("status", out var value), Is.True);
97+
Assert.That(value, Is.EqualTo("approved"));
98+
}
99+
100+
[Test]
101+
public void DuplicateFormKey_WhenSetInAdditionalFormsAndForm_ShouldRetainBothWithSameKey()
102+
{
103+
// Arrange
104+
var parameters = new Parameter.Builder();
105+
var fieldParameters = new Dictionary<string, object> { ["field"] = "additional_form_value", };
106+
107+
parameters
108+
.AdditionalForms(additionalForms => additionalForms.Setup(fieldParameters))
109+
.Form(f => f.Setup("field", "form_value"));
110+
111+
var requestBuilder = new RequestBuilder(LazyGlobalConfiguration.Value, ServerUrl);
112+
113+
// Act
114+
parameters.Validate().Apply(requestBuilder);
115+
116+
// Assert
117+
var formParams = requestBuilder.formParameters
118+
.Where(p => p.Key == "field")
119+
.ToList();
120+
121+
Assert.Multiple(() =>
122+
{
123+
Assert.That(formParams.Count, Is.EqualTo(2), "Expected 2 form parameters with the key 'field'");
124+
Assert.That(formParams.Any(p => Equals(p.Value, "form_value")), Is.True,
125+
"Expected 'form_value' to be present");
126+
Assert.That(formParams.Any(p => Equals(p.Value, "additional_form_value")), Is.True,
127+
"Expected 'additional_form_value' to be present");
128+
});
129+
}
130+
131+
[Test]
132+
public void When_QueryAddedAfterAdditionalQueries_ShouldRetainQueryValues()
133+
{
134+
// Arrange
135+
var parameters = new Parameter.Builder();
136+
parameters.AdditionalQueries(q => q.Setup(AdditionalQueryParams));
137+
foreach (var (key, value) in QueryParams)
138+
parameters.Query(q => q.Setup(key, value));
139+
140+
var requestBuilder = new RequestBuilder(LazyGlobalConfiguration.Value, ServerUrl);
141+
142+
// Act
143+
parameters.Validate().Apply(requestBuilder);
144+
145+
// Assert
146+
Assert.Multiple(() =>
147+
{
148+
foreach (var (key, expectedValue) in QueryParams)
149+
{
150+
Assert.That(requestBuilder.queryParameters.TryGetValue(key, out var actualValue), Is.True,
151+
$"Expected query parameter with key '{key}'");
152+
Assert.That(actualValue, Is.EqualTo(expectedValue),
153+
$"Expected value for key '{key}' to be '{expectedValue}', but was '{actualValue}'");
154+
}
155+
});
156+
}
157+
158+
[Test]
159+
public void When_AdditionalQueriesAddedAfterQuery_ShouldRetainAdditionalQueryValues()
160+
{
161+
// Arrange
162+
var parameters = new Parameter.Builder();
163+
foreach (var (key, value) in QueryParams)
164+
parameters.Query(q => q.Setup(key, value));
165+
parameters.AdditionalQueries(q => q.Setup(AdditionalQueryParams));
166+
167+
var requestBuilder = new RequestBuilder(LazyGlobalConfiguration.Value, ServerUrl);
168+
169+
// Act
170+
parameters.Validate().Apply(requestBuilder);
171+
172+
// Assert
173+
Assert.Multiple(() =>
174+
{
175+
foreach (var (key, expectedValue) in AdditionalQueryParams)
176+
{
177+
Assert.That(requestBuilder.queryParameters.TryGetValue(key, out var actualValue), Is.True,
178+
$"Expected query parameter with key '{key}'");
179+
Assert.That(actualValue, Is.EqualTo(expectedValue),
180+
$"Expected value for key '{key}' to be '{expectedValue}', but was '{actualValue}'");
181+
}
182+
});
183+
}
184+
}
185+
}

‎APIMatic.Core/Request/Parameters/Parameter.cs‎

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@
55
using System.Collections.Concurrent;
66
using System.Collections.Generic;
77
using System.Linq;
8-
using APIMatic.Core.Utilities;
9-
using Microsoft.Json.Pointer;
10-
using Newtonsoft.Json.Linq;
118

129
namespace APIMatic.Core.Request.Parameters
1310
{
@@ -22,9 +19,11 @@ public abstract class Parameter
2219
private Func<object, object> valueSerializer = value => value;
2320
protected bool validated = false;
2421
protected string typeName;
25-
22+
2623
private string GetName() => key == "" ? typeName : key;
2724

25+
private string IdentifierKey => string.IsNullOrEmpty(key) ? $"_{nameof(Parameter)}_{Guid.NewGuid()}" : key;
26+
2827
public Parameter Setup(string key, object value)
2928
{
3029
this.key = key;
@@ -80,7 +79,10 @@ internal virtual void Validate()
8079
/// </summary>
8180
public class Builder
8281
{
83-
private readonly ConcurrentBag<Parameter> parameters = new ConcurrentBag<Parameter>();
82+
private readonly ConcurrentDictionary<string, Parameter> _parameters =
83+
new ConcurrentDictionary<string, Parameter>();
84+
private readonly ConcurrentQueue<string> _insertionOrder = new ConcurrentQueue<string>();
85+
private readonly object _sync = new object();
8486

8587
internal Builder() { }
8688

@@ -93,7 +95,7 @@ public Builder Template(Action<TemplateParam> _template)
9395
{
9496
var template = new TemplateParam();
9597
_template(template);
96-
parameters.Add(template);
98+
AddOrUpdate(template);
9799
return this;
98100
}
99101

@@ -106,7 +108,7 @@ public Builder Header(Action<HeaderParam> _header)
106108
{
107109
var header = new HeaderParam();
108110
_header(header);
109-
parameters.Add(header);
111+
AddOrUpdate(header);
110112
return this;
111113
}
112114

@@ -120,7 +122,7 @@ public Builder AdditionalHeaders(Action<AdditionalHeaderParams> _headers)
120122
{
121123
var headers = new AdditionalHeaderParams();
122124
_headers(headers);
123-
parameters.Add(headers);
125+
AddOrUpdate(headers);
124126
return this;
125127
}
126128

@@ -133,7 +135,7 @@ public Builder Query(Action<QueryParam> _query)
133135
{
134136
var query = new QueryParam();
135137
_query(query);
136-
parameters.Add(query);
138+
AddOrUpdate(query);
137139
return this;
138140
}
139141

@@ -146,7 +148,7 @@ public Builder AdditionalQueries(Action<AdditionalQueryParams> _queries)
146148
{
147149
var queries = new AdditionalQueryParams();
148150
_queries(queries);
149-
parameters.Add(queries);
151+
AddOrUpdate(queries);
150152
return this;
151153
}
152154

@@ -159,7 +161,7 @@ public Builder Form(Action<FormParam> _form)
159161
{
160162
var form = new FormParam();
161163
_form(form);
162-
parameters.Add(form);
164+
AddOrUpdate(form);
163165
return this;
164166
}
165167

@@ -172,7 +174,7 @@ public Builder AdditionalForms(Action<AdditionalFormParams> _forms)
172174
{
173175
var forms = new AdditionalFormParams();
174176
_forms(forms);
175-
parameters.Add(forms);
177+
AddOrUpdate(forms);
176178
return this;
177179
}
178180

@@ -185,7 +187,7 @@ public Builder Body(Action<BodyParam> _body)
185187
{
186188
var body = new BodyParam();
187189
_body(body);
188-
parameters.Add(body);
190+
AddOrUpdate(body);
189191
return this;
190192
}
191193

@@ -196,11 +198,11 @@ public Builder Body(Action<BodyParam> _body)
196198
internal Builder Validate()
197199
{
198200
var missingArgErrors = new List<string>();
199-
foreach (var p in parameters)
201+
foreach (var p in _parameters)
200202
{
201203
try
202204
{
203-
p.Validate();
205+
p.Value.Validate();
204206
}
205207
catch (ArgumentNullException exp)
206208
{
@@ -220,9 +222,25 @@ internal Builder Validate()
220222
/// <returns></returns>
221223
internal void Apply(RequestBuilder requestBuilder)
222224
{
223-
foreach (var p in parameters)
225+
foreach (var p in _insertionOrder)
226+
{
227+
_parameters[p].Apply(requestBuilder);
228+
}
229+
}
230+
231+
private void AddOrUpdate(Parameter param)
232+
{
233+
var key = param.IdentifierKey;
234+
235+
lock (_sync)
224236
{
225-
p.Apply(requestBuilder);
237+
// Only record order on first insertion
238+
if (!_parameters.ContainsKey(key))
239+
{
240+
_insertionOrder.Enqueue(key);
241+
}
242+
243+
_parameters[key] = param;
226244
}
227245
}
228246
}

0 commit comments

Comments
 (0)