Skip to content

Commit cc045fd

Browse files
committed
Address CodeRabbit actionable review feedback
1 parent 480e387 commit cc045fd

9 files changed

Lines changed: 79 additions & 44 deletions

File tree

src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,16 @@ public HtmlConversionBehaviorBuilder AddEmulatedMediaFeature(string name, string
231231
/// </summary>
232232
/// <param name="statusCodes">Validated HTTP status codes.</param>
233233
/// <returns>The builder instance for method chaining.</returns>
234-
public HtmlConversionBehaviorBuilder SetFailOnHttpStatusCodes(IEnumerable<HttpStatusCode> statusCodes)
234+
public HtmlConversionBehaviorBuilder SetFailOnHttpStatusCodes(IEnumerable<GotenbergStatusCode> statusCodes)
235235
{
236236
if (statusCodes == null) throw new ArgumentNullException(nameof(statusCodes));
237237

238-
_htmlConversionBehaviors.FailOnHttpStatusCodes = statusCodes.ToList();
238+
var codes = statusCodes.ToList();
239+
240+
if (codes.Any(c => c == null))
241+
throw new ArgumentException("Status codes collection must not contain null elements.", nameof(statusCodes));
242+
243+
_htmlConversionBehaviors.FailOnHttpStatusCodes = codes;
239244

240245
return this;
241246
}
@@ -247,19 +252,26 @@ public HtmlConversionBehaviorBuilder SetFailOnHttpStatusCodes(IEnumerable<HttpSt
247252
/// <returns>The builder instance for method chaining.</returns>
248253
public HtmlConversionBehaviorBuilder SetFailOnHttpStatusCodes(params int[] statusCodes)
249254
{
250-
return SetFailOnHttpStatusCodes(statusCodes.Select(HttpStatusCode.Create));
255+
if (statusCodes == null) throw new ArgumentNullException(nameof(statusCodes));
256+
257+
return SetFailOnHttpStatusCodes(statusCodes.Select(GotenbergStatusCode.Create));
251258
}
252259

253260
/// <summary>
254261
/// Sets HTTP status codes that trigger a failure when page resources return them.
255262
/// </summary>
256263
/// <param name="statusCodes">Validated HTTP status codes.</param>
257264
/// <returns>The builder instance for method chaining.</returns>
258-
public HtmlConversionBehaviorBuilder SetFailOnResourceHttpStatusCodes(IEnumerable<HttpStatusCode> statusCodes)
265+
public HtmlConversionBehaviorBuilder SetFailOnResourceHttpStatusCodes(IEnumerable<GotenbergStatusCode> statusCodes)
259266
{
260267
if (statusCodes == null) throw new ArgumentNullException(nameof(statusCodes));
261268

262-
_htmlConversionBehaviors.FailOnResourceHttpStatusCodes = statusCodes.ToList();
269+
var codes = statusCodes.ToList();
270+
271+
if (codes.Any(c => c == null))
272+
throw new ArgumentException("Status codes collection must not contain null elements.", nameof(statusCodes));
273+
274+
_htmlConversionBehaviors.FailOnResourceHttpStatusCodes = codes;
263275

264276
return this;
265277
}
@@ -271,7 +283,9 @@ public HtmlConversionBehaviorBuilder SetFailOnResourceHttpStatusCodes(IEnumerabl
271283
/// <returns>The builder instance for method chaining.</returns>
272284
public HtmlConversionBehaviorBuilder SetFailOnResourceHttpStatusCodes(params int[] statusCodes)
273285
{
274-
return SetFailOnResourceHttpStatusCodes(statusCodes.Select(HttpStatusCode.Create));
286+
if (statusCodes == null) throw new ArgumentNullException(nameof(statusCodes));
287+
288+
return SetFailOnResourceHttpStatusCodes(statusCodes.Select(GotenbergStatusCode.Create));
275289
}
276290

277291
/// <summary>
@@ -306,10 +320,12 @@ public HtmlConversionBehaviorBuilder AddIgnoreResourceHttpStatusDomain(string do
306320
/// <returns>The builder instance for method chaining.</returns>
307321
public HtmlConversionBehaviorBuilder AddIgnoreResourceHttpStatusDomains(params string[] domains)
308322
{
309-
foreach (var domain in domains)
310-
{
311-
AddIgnoreResourceHttpStatusDomain(domain);
312-
}
323+
if (domains == null) throw new ArgumentNullException(nameof(domains));
324+
325+
var validated = domains.Select(DomainName.Create).ToList();
326+
327+
_htmlConversionBehaviors.IgnoreResourceHttpStatusDomains ??= new List<DomainName>();
328+
_htmlConversionBehaviors.IgnoreResourceHttpStatusDomains.AddRange(validated);
313329

314330
return this;
315331
}

src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/FacetBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public virtual IEnumerable<HttpContent> ToHttpContent()
8282
List<Cookie> cookies => JsonConvert.SerializeObject(cookies),
8383
List<EmulatedMediaFeature> features => JsonConvert.SerializeObject(
8484
features.ToDictionary(f => f.Name, f => f.Value)),
85-
List<HttpStatusCode> codes => JsonConvert.SerializeObject(codes.Select(c => c.Value)),
85+
List<GotenbergStatusCode> codes => JsonConvert.SerializeObject(codes.Select(c => c.Value)),
8686
List<DomainName> domains => JsonConvert.SerializeObject(domains.Select(d => d.Value)),
8787
CssSelector selector => selector.Value,
8888
float f => f.ToString(cultureInfo),

src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/HtmlConversionBehaviors.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,14 @@ public class HtmlConversionBehaviors : FacetBase
106106
/// when the main page returns a matching code. Default: [499, 599].
107107
/// </summary>
108108
[MultiFormHeader(Constants.Gotenberg.Chromium.Shared.HtmlConvert.FailOnHttpStatusCodes)]
109-
public List<HttpStatusCode>? FailOnHttpStatusCodes { get; set; }
109+
public List<GotenbergStatusCode>? FailOnHttpStatusCodes { get; set; }
110110

111111
/// <summary>
112112
/// HTTP status codes that trigger a failure when page resources (CSS, images, fonts)
113113
/// return a matching code.
114114
/// </summary>
115115
[MultiFormHeader(Constants.Gotenberg.Chromium.Shared.HtmlConvert.FailOnResourceHttpStatusCodes)]
116-
public List<HttpStatusCode>? FailOnResourceHttpStatusCodes { get; set; }
116+
public List<GotenbergStatusCode>? FailOnResourceHttpStatusCodes { get; set; }
117117

118118
/// <summary>
119119
/// Domains to exclude from HTTP status code checks on resources.

src/Gotenberg.Sharp.Api.Client/Domain/ValueObjects/CssSelector.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static CssSelector Create(string selector)
4949

5050
public override int GetHashCode() => Value.GetHashCode();
5151

52-
public static implicit operator string(CssSelector selector) => selector.Value;
52+
public static implicit operator string(CssSelector selector) => selector?.Value!;
5353

5454
public static bool operator ==(CssSelector? left, CssSelector? right) => Equals(left, right);
5555

src/Gotenberg.Sharp.Api.Client/Domain/ValueObjects/DomainName.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public bool Equals(DomainName? other) => other is not null
5050

5151
public override int GetHashCode() => StringComparer.OrdinalIgnoreCase.GetHashCode(Value);
5252

53-
public static implicit operator string(DomainName domain) => domain.Value;
53+
public static implicit operator string(DomainName domain) => domain?.Value!;
5454

5555
public static bool operator ==(DomainName? left, DomainName? right) => Equals(left, right);
5656

src/Gotenberg.Sharp.Api.Client/Domain/ValueObjects/EmulatedMediaFeature.cs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,36 @@ public static EmulatedMediaFeature Create(string name, string value)
5959
return new EmulatedMediaFeature(name, value);
6060
}
6161

62+
private static readonly string[] ValidColorSchemes = { "light", "dark" };
63+
private static readonly string[] ValidReducedMotionValues = { "no-preference", "reduce" };
64+
6265
/// <summary>
6366
/// Creates a "prefers-color-scheme" media feature.
6467
/// </summary>
6568
/// <param name="scheme">The color scheme value: "light" or "dark".</param>
66-
public static EmulatedMediaFeature PrefersColorScheme(string scheme) =>
67-
Create("prefers-color-scheme", scheme);
69+
/// <exception cref="ArgumentException">Thrown when scheme is not "light" or "dark".</exception>
70+
public static EmulatedMediaFeature PrefersColorScheme(string scheme)
71+
{
72+
if (!ValidColorSchemes.Contains(scheme, StringComparer.Ordinal))
73+
throw new ArgumentException(
74+
$"prefers-color-scheme must be 'light' or 'dark', but got '{scheme}'.",
75+
nameof(scheme));
76+
77+
return Create("prefers-color-scheme", scheme);
78+
}
6879

6980
/// <summary>
7081
/// Creates a "prefers-reduced-motion" media feature.
7182
/// </summary>
7283
/// <param name="value">The value: "no-preference" or "reduce".</param>
73-
public static EmulatedMediaFeature PrefersReducedMotion(string value) =>
74-
Create("prefers-reduced-motion", value);
84+
/// <exception cref="ArgumentException">Thrown when value is not "no-preference" or "reduce".</exception>
85+
public static EmulatedMediaFeature PrefersReducedMotion(string value)
86+
{
87+
if (!ValidReducedMotionValues.Contains(value, StringComparer.Ordinal))
88+
throw new ArgumentException(
89+
$"prefers-reduced-motion must be 'no-preference' or 'reduce', but got '{value}'.",
90+
nameof(value));
91+
92+
return Create("prefers-reduced-motion", value);
93+
}
7594
}

src/Gotenberg.Sharp.Api.Client/Domain/ValueObjects/HttpStatusCode.cs renamed to src/Gotenberg.Sharp.Api.Client/Domain/ValueObjects/GotenbergStatusCode.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,14 @@ namespace Gotenberg.Sharp.API.Client.Domain.ValueObjects;
2525
/// Gotenberg uses these as range boundaries. For example, [499, 599] means
2626
/// "fail on any status code from 499 to 599 inclusive."
2727
/// </remarks>
28-
public sealed class HttpStatusCode : IEquatable<HttpStatusCode>, IComparable<HttpStatusCode>
28+
public sealed class GotenbergStatusCode : IEquatable<GotenbergStatusCode>, IComparable<GotenbergStatusCode>
2929
{
3030
public const int MinValue = 100;
3131
public const int MaxValue = 599;
3232

3333
public int Value { get; }
3434

35-
private HttpStatusCode(int value)
35+
private GotenbergStatusCode(int value)
3636
{
3737
Value = value;
3838
}
@@ -42,30 +42,30 @@ private HttpStatusCode(int value)
4242
/// </summary>
4343
/// <param name="statusCode">An HTTP status code between 100 and 599 inclusive.</param>
4444
/// <exception cref="ArgumentOutOfRangeException">Thrown when statusCode is outside valid range.</exception>
45-
public static HttpStatusCode Create(int statusCode)
45+
public static GotenbergStatusCode Create(int statusCode)
4646
{
4747
if (statusCode < MinValue || statusCode > MaxValue)
4848
throw new ArgumentOutOfRangeException(
4949
nameof(statusCode),
5050
statusCode,
5151
$"HTTP status code must be between {MinValue} and {MaxValue}.");
5252

53-
return new HttpStatusCode(statusCode);
53+
return new GotenbergStatusCode(statusCode);
5454
}
5555

5656
public override string ToString() => Value.ToString(CultureInfo.InvariantCulture);
5757

58-
public bool Equals(HttpStatusCode? other) => other is not null && Value == other.Value;
58+
public bool Equals(GotenbergStatusCode? other) => other is not null && Value == other.Value;
5959

60-
public override bool Equals(object? obj) => Equals(obj as HttpStatusCode);
60+
public override bool Equals(object? obj) => Equals(obj as GotenbergStatusCode);
6161

6262
public override int GetHashCode() => Value;
6363

64-
public int CompareTo(HttpStatusCode? other) => other is null ? 1 : Value.CompareTo(other.Value);
64+
public int CompareTo(GotenbergStatusCode? other) => other is null ? 1 : Value.CompareTo(other.Value);
6565

66-
public static implicit operator int(HttpStatusCode code) => code.Value;
66+
public static implicit operator int(GotenbergStatusCode code) => code?.Value ?? 0;
6767

68-
public static bool operator ==(HttpStatusCode? left, HttpStatusCode? right) => Equals(left, right);
68+
public static bool operator ==(GotenbergStatusCode? left, GotenbergStatusCode? right) => Equals(left, right);
6969

70-
public static bool operator !=(HttpStatusCode? left, HttpStatusCode? right) => !Equals(left, right);
70+
public static bool operator !=(GotenbergStatusCode? left, GotenbergStatusCode? right) => !Equals(left, right);
7171
}

test/GotenbergSharpClient.Tests/ChromiumMissingFieldsTests.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,10 @@ public async Task FailOnHttpStatusCodes_SerializesToIntArray()
189189
{
190190
var behaviors = new HtmlConversionBehaviors
191191
{
192-
FailOnHttpStatusCodes = new List<HttpStatusCode>
192+
FailOnHttpStatusCodes = new List<GotenbergStatusCode>
193193
{
194-
HttpStatusCode.Create(499),
195-
HttpStatusCode.Create(599)
194+
GotenbergStatusCode.Create(499),
195+
GotenbergStatusCode.Create(599)
196196
}
197197
};
198198

@@ -214,10 +214,10 @@ public async Task FailOnResourceHttpStatusCodes_SerializesToIntArray()
214214
{
215215
var behaviors = new HtmlConversionBehaviors
216216
{
217-
FailOnResourceHttpStatusCodes = new List<HttpStatusCode>
217+
FailOnResourceHttpStatusCodes = new List<GotenbergStatusCode>
218218
{
219-
HttpStatusCode.Create(400),
220-
HttpStatusCode.Create(500)
219+
GotenbergStatusCode.Create(400),
220+
GotenbergStatusCode.Create(500)
221221
}
222222
};
223223

test/GotenbergSharpClient.Tests/ValueObjects/HttpStatusCodeTests.cs renamed to test/GotenbergSharpClient.Tests/ValueObjects/GotenbergStatusCodeTests.cs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
namespace GotenbergSharpClient.Tests.ValueObjects;
44

55
[TestFixture]
6-
public class HttpStatusCodeTests
6+
public class GotenbergStatusCodeTests
77
{
88
[TestCase(100)]
99
[TestCase(200)]
@@ -12,7 +12,7 @@ public class HttpStatusCodeTests
1212
[TestCase(599)]
1313
public void Create_WithValidStatusCode_ReturnsInstance(int code)
1414
{
15-
var statusCode = HttpStatusCode.Create(code);
15+
var statusCode = GotenbergStatusCode.Create(code);
1616

1717
statusCode.Value.Should().Be(code);
1818
}
@@ -24,15 +24,15 @@ public void Create_WithValidStatusCode_ReturnsInstance(int code)
2424
[TestCase(1000)]
2525
public void Create_WithOutOfRangeCode_ThrowsArgumentOutOfRangeException(int code)
2626
{
27-
var act = () => HttpStatusCode.Create(code);
27+
var act = () => GotenbergStatusCode.Create(code);
2828

2929
act.Should().ThrowExactly<ArgumentOutOfRangeException>();
3030
}
3131

3232
[Test]
3333
public void ImplicitConversion_ToIntReturnsValue()
3434
{
35-
var statusCode = HttpStatusCode.Create(404);
35+
var statusCode = GotenbergStatusCode.Create(404);
3636
int result = statusCode;
3737

3838
result.Should().Be(404);
@@ -41,8 +41,8 @@ public void ImplicitConversion_ToIntReturnsValue()
4141
[Test]
4242
public void Equals_WithSameValue_ReturnsTrue()
4343
{
44-
var a = HttpStatusCode.Create(499);
45-
var b = HttpStatusCode.Create(499);
44+
var a = GotenbergStatusCode.Create(499);
45+
var b = GotenbergStatusCode.Create(499);
4646

4747
a.Should().Be(b);
4848
(a == b).Should().BeTrue();
@@ -51,8 +51,8 @@ public void Equals_WithSameValue_ReturnsTrue()
5151
[Test]
5252
public void CompareTo_OrdersCorrectly()
5353
{
54-
var low = HttpStatusCode.Create(200);
55-
var high = HttpStatusCode.Create(500);
54+
var low = GotenbergStatusCode.Create(200);
55+
var high = GotenbergStatusCode.Create(500);
5656

5757
low.CompareTo(high).Should().BeNegative();
5858
high.CompareTo(low).Should().BePositive();
@@ -62,7 +62,7 @@ public void CompareTo_OrdersCorrectly()
6262
[Test]
6363
public void ToString_ReturnsInvariantString()
6464
{
65-
var statusCode = HttpStatusCode.Create(404);
65+
var statusCode = GotenbergStatusCode.Create(404);
6666

6767
statusCode.ToString().Should().Be("404");
6868
}

0 commit comments

Comments
 (0)