Skip to content

Conversation

Jaben
Copy link
Member

@Jaben Jaben commented Oct 5, 2025

Moved the project from /lib to /src/Gotenberg so more then one library can be in this solution.

Summary by CodeRabbit

  • New Features

    • Fluent builders to compose documents and configure HTML→PDF conversions.
    • Cookie support, custom headers, metadata, wait delays/expressions, media emulation, network-idle control, PDF/A format selection, and PDF/UA enablement.
    • Option to fail on browser console exceptions.
  • Chores

    • Removed legacy AppVeyor CI.
    • Project restructured for multi-targeting and updated packaging/source-linking.
  • Tests

    • Updated test project references.
  • Style

    • Formatting and header cleanups.

Copy link
Contributor

coderabbitai bot commented Oct 5, 2025

Warning

Rate limit exceeded

@Jaben has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 20 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between fff69d1 and e9536b8.

📒 Files selected for processing (4)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilderExtensions.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/WebhookBuilder.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Moves project files into a new src layout and adds a multi-targeted project. Removes AppVeyor CI. Introduces Facet infrastructure (FacetBase), HTML conversion DTO and fluent builder (HtmlConversionBehaviors, HtmlConversionBehaviorBuilder + extensions), a Cookie record with validation, a DocumentBuilder, and a comprehensive Constants catalog. Some files only formatted.

Changes

Cohort / File(s) Summary
Solution & test paths
GotenbergSharpApiClient.sln, test/GotenbergSharpClient.Tests/GotenbergSharpClient.Tests.csproj, src/Gotenberg.Sharp.Api.Client/GotenbergSharpApiClient.DotSettings.sln
Update solution/test references to src/... project path; whitespace tweak in DotSettings.
CI removal
appveyor.yml
Remove AppVeyor configuration (Windows CI build/pack/push pipeline).
Project configuration
src/Gotenberg.Sharp.Api.Client/Gotenberg.Sharp.Api.Client.csproj
Add new multi-targeted MSBuild project with packaging metadata, SourceLink/PolySharp, conditional framework dependencies, and symbol/doc packaging settings.
Facet infrastructure & DTOs
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/FacetBase.cs, .../HtmlConversionBehaviors.cs, .../Cookie.cs
Add abstract FacetBase with ToHttpContent mapping, HtmlConversionBehaviors DTO (mapped MultiForm headers), and new Cookie record with Validate method.
HTML conversion builders
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs, .../HtmlConversionBehaviorBuilderExtensions.cs
Add fluent HtmlConversionBehaviorBuilder and extension methods to configure wait, headers, cookies, metadata, PDF format/UA, and other flags.
Document composition builder
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/DocumentBuilder.cs
Add DocumentBuilder for assembling FullDocument body/header/footer and markdown flag.
Constants catalog
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs
Add hierarchical, centralized string constants for routes, headers, media types, multipart/disposition, and Chromium/LibreOffice/PDF options.
Formatting-only edits
src/Gotenberg.Sharp.Api.Client/Domain/Builders/MergeOfficeBuilder.cs, src/Gotenberg.Sharp.Api.Client/Domain/Requests/MergeOfficeRequest.cs
License/header and whitespace reformatting; no behavioral changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as Client Code
  participant DocB as DocumentBuilder
  participant HtmlB as HtmlConversionBehaviorBuilder
  participant Facet as HtmlConversionBehaviors (Facet)
  participant FB as FacetBase
  participant HTTP as HttpClient
  participant API as Gotenberg API

  App->>DocB: SetBody/Header/Footer(...)
  App->>HtmlB: Configure wait/headers/cookies/metadata/pdf...
  HtmlB->>Facet: Populate HtmlConversionBehaviors
  App->>FB: Request ToHttpContent() (includes Facet properties + Files)
  FB->>HTTP: Send multipart/form-data (mapped properties, cookies, headers)
  HTTP->>API: POST /chromium/convertHtml (or convertUrl)
  API-->>HTTP: PDF response / error
  HTTP-->>App: Return PDF stream or error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

I nibble bytes in tidy rows,
New facets sprout where data flows.
Cookies slipped into tiny jars,
Builders stack like tiny stars.
Constants guide the API sea—carrots, docs, and PDFs for me! 🥕📄

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the directory relocation from “lib” to “src”, which is a real aspect of this changeset though it omits the broader additions of new builder classes and infrastructure constants.
Docstring Coverage ✅ Passed Docstring coverage is 92.86% which is sufficient. The required threshold is 80.00%.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 490797c and 6f06b85.

⛔ Files ignored due to path filters (2)
  • src/Gotenberg.Sharp.Api.Client/Resources/gotenbergSharpClient-large.PNG is excluded by !**/*.png
  • src/Gotenberg.Sharp.Api.Client/Resources/gotenbergSharpClient.PNG is excluded by !**/*.png
📒 Files selected for processing (14)
  • GotenbergSharpApiClient.sln (1 hunks)
  • appveyor.yml (0 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/DocumentBuilder.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilderExtensions.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/MergeOfficeBuilder.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/Cookie.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/FacetBase.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/HtmlConversionBehaviors.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Domain/Requests/MergeOfficeRequest.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Gotenberg.Sharp.Api.Client.csproj (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/GotenbergSharpApiClient.DotSettings.sln (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1 hunks)
  • test/GotenbergSharpClient.Tests/GotenbergSharpClient.Tests.csproj (1 hunks)
💤 Files with no reviewable changes (1)
  • appveyor.yml
🧰 Additional context used
🧬 Code graph analysis (11)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/MergeOfficeRequest.cs (6)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (14)
  • Gotenberg (51-295)
  • Constants (18-296)
  • LibreOffice (97-123)
  • ApiPaths (57-64)
  • ApiPaths (99-104)
  • ApiPaths (130-137)
  • ApiPaths (177-186)
  • HttpContent (23-49)
  • Routes (106-122)
  • Routes (139-150)
  • Routes (188-209)
  • Convert (108-121)
  • Convert (146-149)
  • Merge (141-144)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/PdfRequestBase.cs (5)
  • PdfRequestBase (18-73)
  • IEnumerable (64-72)
  • HttpContent (32-40)
  • HttpContent (42-50)
  • HttpContent (52-62)
src/Gotenberg.Sharp.Api.Client/Infrastructure/ContentTypes/ResolveContentTypeImplementation.cs (1)
  • ResolveContentTypeImplementation (22-33)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/AssetDictionary.cs (1)
  • IEnumerable (25-51)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/MergeOfficeConstants.cs (1)
  • MergeOfficeConstants (18-40)
src/Gotenberg.Sharp.Api.Client/Extensions/StringExtensions.cs (1)
  • IsSet (20-23)
GotenbergSharpApiClient.sln (1)
lib/GotenbergSharpClient.cs (4)
  • Task (186-193)
  • Task (195-205)
  • GotenbergSharpClient (37-319)
  • GotenbergSharpClient (56-70)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/DocumentBuilder.cs (4)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/FullDocument.cs (1)
  • FullDocument (22-26)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/ContentItem.cs (5)
  • ContentItem (20-52)
  • ContentItem (24-27)
  • ContentItem (29-33)
  • ContentItem (35-40)
  • ContentItem (42-46)
lib/Domain/Builders/HtmlRequestBuilder.cs (3)
  • HtmlRequestBuilder (18-44)
  • HtmlRequestBuilder (26-33)
  • HtmlRequestBuilder (35-43)
lib/Domain/Requests/Facets/HeaderFooterDocument.cs (1)
  • HeaderFooterDocument (18-25)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/MergeOfficeBuilder.cs (4)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1)
  • Gotenberg (51-295)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/BaseMergeBuilder.cs (1)
  • BaseMergeBuilder (18-38)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/MergeOfficeRequest.cs (1)
  • MergeOfficeRequest (24-67)
src/Gotenberg.Sharp.Api.Client/Domain/Pages/PageRanges.cs (3)
  • PageRanges (21-119)
  • PageRanges (26-29)
  • PageRanges (40-75)
src/Gotenberg.Sharp.Api.Client/Gotenberg.Sharp.Api.Client.csproj (2)
lib/GotenbergSharpClient.cs (8)
  • GotenbergSharpClient (37-319)
  • Task (195-205)
  • Task (186-193)
  • GotenbergSharpClient (56-70)
  • Task (301-318)
  • Task (244-253)
  • Task (270-292)
  • Task (147-154)
lib/Domain/Settings/GotenbergSharpClientOptions.cs (1)
  • GotenbergSharpClientOptions (18-45)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/HtmlConversionBehaviors.cs (4)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (5)
  • Gotenberg (51-295)
  • Constants (18-296)
  • Chromium (175-294)
  • Shared (211-293)
  • HtmlConvert (257-285)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/FacetBase.cs (1)
  • FacetBase (20-89)
lib/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (2)
  • HtmlConversionBehaviorBuilder (20-229)
  • HtmlConversionBehaviorBuilder (195-200)
lib/Domain/Requests/ChromeRequest.cs (2)
  • Gotenberg (16-29)
  • ChromeRequest (18-28)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/Cookie.cs (3)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1)
  • Gotenberg (51-295)
src/Gotenberg.Sharp.Api.Client/Extensions/StringExtensions.cs (1)
  • IsNotSet (25-28)
lib/Domain/Requests/Facets/Cookie.cs (2)
  • Cookie (24-74)
  • Validate (68-73)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/FacetBase.cs (6)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (3)
  • Gotenberg (51-295)
  • HttpContent (23-49)
  • Headers (25-28)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/BuildRequestBase.cs (2)
  • IEnumerable (39-39)
  • StringContent (30-37)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/IConvertToHttpContent.cs (1)
  • IEnumerable (20-20)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/ContentItem.cs (6)
  • HttpContent (48-51)
  • ContentItem (20-52)
  • ContentItem (24-27)
  • ContentItem (29-33)
  • ContentItem (35-40)
  • ContentItem (42-46)
src/Gotenberg.Sharp.Api.Client/Infrastructure/MultiFormPropertyItem.cs (1)
  • MultiFormPropertyItem (20-45)
src/Gotenberg.Sharp.Api.Client/Extensions/EnumExtensions.cs (2)
  • ToFormDataValue (24-27)
  • ToFormDataValue (29-32)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (5)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1)
  • Gotenberg (51-295)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilderExtensions.cs (2)
  • HtmlConversionBehaviorBuilder (32-58)
  • HtmlConversionBehaviorBuilder (67-85)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/HtmlConversionBehaviors.cs (1)
  • HtmlConversionBehaviors (23-103)
src/Gotenberg.Sharp.Api.Client/Extensions/StringExtensions.cs (1)
  • IsNotSet (25-28)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/Cookie.cs (1)
  • Validate (68-73)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (6)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/PdfRequestBase.cs (3)
  • HttpContent (32-40)
  • HttpContent (42-50)
  • HttpContent (52-62)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/UrlRequest.cs (2)
  • HttpContent (43-51)
  • HttpContent (53-63)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/Webhook.cs (1)
  • Webhook (18-109)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/PageProperties.cs (4)
  • PageProperties (28-174)
  • PageProperties (30-43)
  • PageProperties (150-151)
  • PageProperties (160-171)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/ExtraHttpHeaders.cs (1)
  • ExtraHttpHeaders (18-50)
lib/GotenbergSharpClient.cs (2)
  • GotenbergSharpClient (37-319)
  • GotenbergSharpClient (56-70)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilderExtensions.cs (2)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1)
  • Gotenberg (51-295)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (14)
  • HtmlConversionBehaviorBuilder (20-229)
  • HtmlConversionBehaviorBuilder (24-27)
  • HtmlConversionBehaviorBuilder (35-40)
  • HtmlConversionBehaviorBuilder (50-60)
  • HtmlConversionBehaviorBuilder (89-94)
  • HtmlConversionBehaviorBuilder (102-112)
  • HtmlConversionBehaviorBuilder (120-134)
  • HtmlConversionBehaviorBuilder (143-148)
  • HtmlConversionBehaviorBuilder (157-167)
  • HtmlConversionBehaviorBuilder (173-178)
  • HtmlConversionBehaviorBuilder (184-189)
  • HtmlConversionBehaviorBuilder (195-200)
  • HtmlConversionBehaviorBuilder (208-218)
  • HtmlConversionBehaviorBuilder (223-228)
🪛 GitHub Actions: Build and Push to Nuget
src/Gotenberg.Sharp.Api.Client/Gotenberg.Sharp.Api.Client.csproj

[error] 1-1: dotnet pack failed. NU5019: File not found: '/home/runner/work/GotenbergSharpApiClient/GotenbergSharpApiClient/src/README.md'.


[warning] 221-221: NU5118: File '/home/runner/work/GotenbergSharpApiClient/GotenbergSharpApiClient/src/Gotenberg.Sharp.Api.Client/Resources/gotenbergSharpClient-large.PNG' is not added because the package already contains file '/gotenbergSharpClient-large.PNG'.

🪛 GitHub Check: build (8.x)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs

[warning] 214-214:
XML comment has badly formed XML -- 'Whitespace is not allowed at this location.'


[warning] 214-214:
XML comment has badly formed XML -- 'Whitespace is not allowed at this location.'


[warning] 214-214:
XML comment has badly formed XML -- 'Whitespace is not allowed at this location.'


[warning] 214-214:
XML comment has badly formed XML -- 'Whitespace is not allowed at this location.'

🔇 Additional comments (5)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/HtmlConversionBehaviors.cs (1)

23-103: LGTM! Well-structured DTO with comprehensive documentation.

The class is well-designed with:

  • Clear property names matching Gotenberg API parameters
  • Thorough XML documentation with remarks and examples
  • Appropriate use of nullable types for optional settings
  • Correct obsolete marking for deprecated UserAgent property
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (3)

102-112: Consider merging behavior for headers.

The current implementation replaces all extra headers when called multiple times. Verify this is the intended behavior, as users might expect headers to accumulate.

If accumulation is desired, consider this pattern:

     public HtmlConversionBehaviorBuilder AddAdditionalHeaders(JObject extraHeaders)
     {
         if (extraHeaders == null)
         {
             throw new InvalidOperationException("extraHeaders is null");
         }

+        _htmlConversionBehaviors.ExtraHeaders ??= new JObject();
+        foreach (var prop in extraHeaders.Properties())
+        {
+            _htmlConversionBehaviors.ExtraHeaders[prop.Name] = prop.Value;
+        }
-        _htmlConversionBehaviors.ExtraHeaders = extraHeaders;

         return this;
     }

157-167: Consider merging behavior for metadata.

Similar to headers, the current implementation replaces all metadata when called multiple times. Verify this is the intended behavior.

If accumulation is desired, apply a similar merge pattern as suggested for headers.


35-40: LGTM! Builder methods follow consistent patterns.

The remaining builder methods are well-implemented with:

  • Appropriate validation where needed
  • Consistent return-this pattern for fluent chaining
  • Clear intent and naming

Also applies to: 50-60, 120-134, 173-178, 184-189, 195-200, 208-218, 223-228

src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1)

18-296: LGTM! Well-organized constants catalog.

The constants file is excellently structured with:

  • Clear hierarchical organization matching the Gotenberg API
  • Good use of nested classes for grouping related constants
  • Appropriate visibility modifiers (private for internal CrossCutting class)
  • Helpful XML documentation and remarks
  • Consistent naming conventions

Comment on lines +27 to +31
public DocumentBuilder(FullDocument content, Action<bool> setContainsMarkdown)
{
this._content = content ?? throw new ArgumentNullException(nameof(content));
this._setContainsMarkdown = setContainsMarkdown;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Null-guard the markdown callback.

If a consumer accidentally passes null for setContainsMarkdown, the next call to SetContainsMarkdown() or ContainsMarkdown() will throw a NullReferenceException. Fail fast with an ArgumentNullException instead.

-        this._content = content ?? throw new ArgumentNullException(nameof(content));
-        this._setContainsMarkdown = setContainsMarkdown;
+        this._content = content ?? throw new ArgumentNullException(nameof(content));
+        this._setContainsMarkdown =
+            setContainsMarkdown ?? throw new ArgumentNullException(nameof(setContainsMarkdown));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public DocumentBuilder(FullDocument content, Action<bool> setContainsMarkdown)
{
this._content = content ?? throw new ArgumentNullException(nameof(content));
this._setContainsMarkdown = setContainsMarkdown;
}
public DocumentBuilder(FullDocument content, Action<bool> setContainsMarkdown)
{
this._content = content ?? throw new ArgumentNullException(nameof(content));
this._setContainsMarkdown =
setContainsMarkdown ?? throw new ArgumentNullException(nameof(setContainsMarkdown));
}
🤖 Prompt for AI Agents
In src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/DocumentBuilder.cs
around lines 27 to 31, the constructor currently null-checks only the content
parameter but not the setContainsMarkdown callback; add a null-guard for
setContainsMarkdown and throw an
ArgumentNullException(nameof(setContainsMarkdown)) when it is null, then assign
the field as before so any later calls to SetContainsMarkdown() or
ContainsMarkdown() fail fast with a clear exception instead of a
NullReferenceException.

Comment on lines 89 to 94
public HtmlConversionBehaviorBuilder AddAdditionalHeaders(string headerName, string headerValue)
{
var header = string.Format("{0}{2}{1}", "{", "}", $"{'"'}{headerName}{'"'} : {'"'}{headerValue}{'"'}");

return AddAdditionalHeaders(JObject.Parse(header));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Avoid manual JSON string construction.

The current approach uses string formatting to build JSON, which is fragile and prone to errors (e.g., missing escaping for special characters in values).

Apply this diff to use JObject construction directly:

     public HtmlConversionBehaviorBuilder AddAdditionalHeaders(string headerName, string headerValue)
     {
-        var header = string.Format("{0}{2}{1}", "{", "}", $"{'"'}{headerName}{'"'} : {'"'}{headerValue}{'"'}");
-
-        return AddAdditionalHeaders(JObject.Parse(header));
+        return AddAdditionalHeaders(new JObject { [headerName] = headerValue });
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public HtmlConversionBehaviorBuilder AddAdditionalHeaders(string headerName, string headerValue)
{
var header = string.Format("{0}{2}{1}", "{", "}", $"{'"'}{headerName}{'"'} : {'"'}{headerValue}{'"'}");
return AddAdditionalHeaders(JObject.Parse(header));
}
public HtmlConversionBehaviorBuilder AddAdditionalHeaders(string headerName, string headerValue)
{
return AddAdditionalHeaders(new JObject { [headerName] = headerValue });
}
🤖 Prompt for AI Agents
In
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs
around lines 89 to 94, avoid manual JSON string construction: instead of
building a JSON string and parsing it, construct a JObject directly (e.g., new
JObject { [headerName] = headerValue }) and pass that to AddAdditionalHeaders;
this ensures proper escaping and avoids fragile string formatting.

Comment on lines 105 to 110
<ItemGroup>
<None Include="..\README.md" Pack="true" PackagePath="\" />
<None Include="..\.editorconfig" Link=".editorconfig" />
<None Include="Resources\gotenbergSharpClient-large.PNG" Pack="true" PackagePath="\" />
<Resource Include="Resources\gotenbergSharpClient-large.PNG" Pack="true" PackagePath="\" />
</ItemGroup>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix README path to unblock NuGet packing

dotnet pack is failing with NU5019 because ..\README.md now resolves to src/README.md, which doesn't exist after moving the project under src/. Update the relative path to reach the repository root (e.g., ..\..\README.md) so packing succeeds.

-    <None Include="..\README.md" Pack="true" PackagePath="\" />
+    <None Include="..\..\README.md" Pack="true" PackagePath="\" />
🤖 Prompt for AI Agents
In src/Gotenberg.Sharp.Api.Client/Gotenberg.Sharp.Api.Client.csproj around lines
105 to 110, the README path used for packing points to ..\README.md which now
resolves to src/README.md and doesn't exist after moving the project; update the
relative path to point to the repository root (for example change to
..\..\README.md) so dotnet pack can locate the README and NU5019 is resolved,
and verify any other file references in the ItemGroup have correct relative
paths after the move.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (1)

90-94: Stop manually composing JSON headers.

Formatting JSON with string.Format breaks as soon as headerName or headerValue contains quotes, braces, or other JSON-reserved characters—leading to FormatException/JsonReaderException and unparseable payloads. Use JObject construction so inputs are properly escaped and structured.

     public HtmlConversionBehaviorBuilder AddAdditionalHeaders(string headerName, string headerValue)
     {
-        var header = string.Format("{0}{2}{1}", "{", "}", $"{'"'}{headerName}{'"'} : {'"'}{headerValue}{'"'}");
-
-        return AddAdditionalHeaders(JObject.Parse(header));
+        var header = new JObject { [headerName] = headerValue };
+
+        return AddAdditionalHeaders(header);
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f06b85 and fff69d1.

📒 Files selected for processing (2)
  • src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (1 hunks)
  • src/Gotenberg.Sharp.Api.Client/Gotenberg.Sharp.Api.Client.csproj (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Gotenberg.Sharp.Api.Client/Gotenberg.Sharp.Api.Client.csproj
🧰 Additional context used
🧬 Code graph analysis (1)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilder.cs (5)
src/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cs (1)
  • Gotenberg (51-295)
src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/HtmlConversionBehaviorBuilderExtensions.cs (2)
  • HtmlConversionBehaviorBuilder (32-58)
  • HtmlConversionBehaviorBuilder (67-85)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/HtmlConversionBehaviors.cs (1)
  • HtmlConversionBehaviors (23-103)
src/Gotenberg.Sharp.Api.Client/Extensions/StringExtensions.cs (1)
  • IsNotSet (25-28)
src/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/Cookie.cs (1)
  • Validate (68-73)

@Jaben Jaben merged commit 39d6d68 into develop Oct 5, 2025
3 checks passed
@Jaben Jaben deleted the feature/Lib-to-Src branch October 5, 2025 17:23
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.

1 participant