Skip to content

feat: add feature for handling multiple image source types #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

osm-Jatin
Copy link

@osm-Jatin osm-Jatin commented Jun 21, 2025

Task Link

REST-1584

Description

  • add separate ImageData class
  • add the feature of handling different types of images
  • update the controller according to the separate handling of images

Summary by CodeRabbit

  • New Features

    • Introduced authentication endpoints for login and token revocation using JWT tokens stored in Redis.
    • Added PDF and Word document generation from templates with support for placeholders, tables, and images.
    • Added Redis integration with new environment configuration variables.
    • Introduced Redis token store service and interface for token management.
    • Added new data models and enums for document content, tables, images, and placeholders.
  • Improvements

    • Enhanced configuration validation and error handling in document generation APIs.
    • Updated all project references, namespaces, and documentation to "OsmoDoc".
    • Improved request validation with data annotations and refined error responses.
    • Updated Swagger docs and JWT authentication to support token revocation checks.
    • Simplified document generation logic by removing legacy code and streamlining image handling.
  • Bug Fixes

    • Corrected project and documentation links to reflect updated repository and project names.
  • Chores

    • Refactored namespaces and project structure from "DocumentService" to "OsmoDoc".
    • Updated Docker and docker-compose setups to include Redis and rename services accordingly.
    • Added new project files while removing obsolete code and configuration files.
    • Strengthened environment variable placeholders for better security guidance.

osm-Jatin and others added 30 commits June 15, 2025 14:51
@osm-Jatin osm-Jatin requested a review from sameer-s-b June 21, 2025 19:22
Copy link

coderabbitai bot commented Jun 21, 2025

Warning

Rate limit exceeded

@osm-Jatin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 0 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 628e093 and fff1e69.

📒 Files selected for processing (2)
  • OsmoDoc.API/Controllers/LoginController.cs (1 hunks)
  • OsmoDoc/Pdf/PdfDocumentGenerator.cs (1 hunks)

Walkthrough

This update transitions the project from "DocumentService" to "OsmoDoc," renaming namespaces, files, and configuration throughout. It introduces Redis-based JWT token management, adds new API endpoints for login and token revocation, and refactors DTOs and document generation logic into the new OsmoDoc structure. Docker and documentation are updated accordingly.

Changes

Files / Groups Change Summary
.env.example Updated JWT_KEY placeholder, added REDIS_URL and REDIS_PORT variables.
.github/PULL_REQUEST_TEMPLATE/...
CONTRIBUTING.md
README.md
Updated project/repo references from "document-service" to "osmodoc".
Dockerfile
docker-compose.yaml
Renamed service to osmodoc, added Redis service, updated file/project references.
DocumentService.* (API, Models, Helpers, Word, Pdf, csproj) Deleted old project files, DTOs, helpers, and document generation logic.
OsmoDoc.* (API, Models, Helpers, Pdf, Word, Services, csproj, sln) Added/renamed files for OsmoDoc, including new DTOs, helpers, document generators, Redis token store, and project/solution files.
OsmoDoc.API/Controllers/* Refactored controllers to OsmoDoc namespace, added LoginController, updated Pdf/Word controllers for new DTOs and async logic.
OsmoDoc.API/Program.cs Added Redis connection, token store, PDF tool config, JWT revocation check, updated Swagger title.
docs/site/* Updated documentation and cross-references from "DocumentService" to "OsmoDoc".
manifest.json
xrefmap.yml
Updated documentation manifest and cross-reference map for new namespaces/paths.

Sequence Diagram(s)

Login and Token Management Flow

sequenceDiagram
    participant Client
    participant LoginController
    participant AuthenticationHelper
    participant RedisTokenStoreService

    Client->>LoginController: POST /api/login (email)
    LoginController->>AuthenticationHelper: Generate JWT Token
    AuthenticationHelper-->>LoginController: JWT Token
    LoginController->>RedisTokenStoreService: StoreTokenAsync(token, email)
    RedisTokenStoreService-->>LoginController: (ack)
    LoginController-->>Client: BaseResponse (token, success)

    Client->>LoginController: POST /api/revoke (token)
    LoginController->>RedisTokenStoreService: RevokeTokenAsync(token)
    RedisTokenStoreService-->>LoginController: (ack)
    LoginController-->>Client: BaseResponse (success)
Loading

PDF Generation Flow

sequenceDiagram
    participant Client
    participant PdfController
    participant PdfDocumentGenerator

    Client->>PdfController: POST /api/pdf (PdfGenerationRequestDTO)
    PdfController->>PdfDocumentGenerator: GeneratePdf(templatePath, metaData, outputPath, isEjs, ejsJson)
    PdfDocumentGenerator->>PdfDocumentGenerator: (Convert EJS to HTML if needed)
    PdfDocumentGenerator->>PdfDocumentGenerator: (Replace placeholders in HTML)
    PdfDocumentGenerator->>PdfDocumentGenerator: (Convert HTML to PDF)
    PdfDocumentGenerator-->>PdfController: (PDF generated)
    PdfController-->>Client: BaseResponse (success, PDF path)
Loading

Word Document Generation Flow

sequenceDiagram
    participant Client
    participant WordController
    participant WordDocumentGenerator

    Client->>WordController: POST /api/word (WordGenerationRequestDTO)
    WordController->>WordDocumentGenerator: GenerateDocumentByTemplate(templatePath, documentData, outputPath)
    WordDocumentGenerator->>WordDocumentGenerator: (Replace text/table/image placeholders)
    WordDocumentGenerator-->>WordController: (Word doc generated)
    WordController-->>Client: BaseResponse (success, Word doc path)
Loading

Poem

🐇✨
OsmoDoc leaps from fields anew,
With Redis tokens, strong and true.
PDF and Word now swiftly flow,
Through refactored names, we onward go!
Documentation crisp and bright,
Docker hops set up just right—
The code now shines in OsmoDoc light!

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@osm-Jatin osm-Jatin self-assigned this Jun 21, 2025
Copy link

@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: 20

🧹 Nitpick comments (21)
OsmoDoc/Word/Models/ImageData.cs (2)

3-8: Consider adding XML documentation for enum values.

The enum values would benefit from XML documentation to clarify their intended usage, especially the difference between LocalFile and Url.

+/// <summary>
+/// Specifies the type of image source.
+/// </summary>
 public enum ImageSourceType
 {
+    /// <summary>Base64 encoded image data.</summary>
     Base64,
+    /// <summary>Local file system path.</summary>
     LocalFile,
+    /// <summary>Remote URL to image resource.</summary>
     Url
 }

10-23: Consider security implications for URL and LocalFile sources.

The Data property will contain file paths or URLs that could pose security risks if not properly validated. Consider implementing additional validation for:

  • Path traversal attacks for LocalFile sources
  • URL validation and allowed domains for Url sources
  • File extension validation for LocalFile sources

Would you like me to help generate validation attributes or helper methods to secure these different source types?

.env.example (1)

3-4: Clarify Redis configuration

Consider splitting REDIS_URL into host-only (e.g., redis) and using REDIS_PORT for the port, or renaming REDIS_URL to REDIS_HOST to avoid ambiguity when constructing the connection string.

OsmoDoc/Pdf/Models/DocumentData.cs (2)

1-4: Remove unused using directives.
Neither System nor System.Text is referenced; dropping them will clean up the code.


7-10: Consider adding XML documentation.
Public model DocumentData and its Placeholders property would benefit from <summary> comments for clarity and maintainability.

Dockerfile (1)

17-18: Simplify project restore paths
Remove redundant ./ segments for clarity:

-RUN dotnet restore "./OsmoDoc.API/./OsmoDoc.API.csproj"
+RUN dotnet restore "./OsmoDoc.API/OsmoDoc.API.csproj"
-RUN dotnet restore "./OsmoDoc/./OsmoDoc.csproj"
+RUN dotnet restore "./OsmoDoc/OsmoDoc.csproj"
README.md (3)

27-27: Unify project name casing in instructions
Use a consistent casing (OsmoDoc) or full repository path (OsmosysSoftware/osmodoc) when telling users to clone or dockerize the project.

Also applies to: 52-52


103-105: Refine phrasing for readability
Change "Go to the location to the bin files of your project" to something like "Go to your project's bin directory" to avoid redundancy.


190-190: Remove unnecessary article
Adjust "The OsmoDoc is licensed under..." to "OsmoDoc is licensed under..." for smoother phrasing.

OsmoDoc.API/Models/RevokeTokenRequestDTO.cs (1)

5-9: Well-structured DTO with appropriate validation.

The implementation is clean with proper validation attributes and initialization. Consider adding token format validation if specific token patterns are expected.

For enhanced security, consider adding token format validation:

public class RevokeTokenRequestDTO
{
    [Required(ErrorMessage = "Token is required")]
+    [StringLength(2048, MinimumLength = 10, ErrorMessage = "Token length is invalid")]
    public string Token { get; set; } = string.Empty;
}
OsmoDoc.API/Models/LoginRequestDTO.cs (1)

7-9: Consider additional security measures for authentication.

The validation attributes are appropriate, but consider these security enhancements:

  • Add rate limiting to prevent brute force attacks on the login endpoint
  • Consider adding additional fields like password or token for proper authentication
  • Ensure the Email property is properly sanitized before database queries to prevent injection attacks
OsmoDoc/Word/Models/Enums.cs (1)

18-30: Consider adding descriptive comments for enum usage.

While the XML documentation is good, consider adding brief comments about when each ParentBody value should be used in practice.

 /// <summary>
 /// Represents the parent body of a placeholder in a Word document.
 /// </summary>
 public enum ParentBody
 {
     /// <summary>
-    /// The placeholder does not have a parent body.
+    /// The placeholder does not have a parent body (e.g., placeholders in document body).
     /// </summary>
     None = 0,

     /// <summary>
-    /// The placeholder belongs to a table.
+    /// The placeholder belongs to a table (e.g., table cell placeholders).
     /// </summary>
     Table = 1
 }
OsmoDoc.API/Models/PdfGenerationRequestDTO.cs (1)

11-11: Consider adding validation or documentation for SerializedEjsDataJson.

While the property is optional, consider adding XML documentation to clarify its expected format and usage, or validation to ensure it's valid JSON when provided.

+    /// <summary>
+    /// Optional serialized JSON data for EJS template processing.
+    /// </summary>
     public string? SerializedEjsDataJson { get; set; }
OsmoDoc/Services/Interfaces/IRedisTokenStoreService.cs (1)

6-11: Add XML documentation for the interface.

The interface methods have clear names, but adding XML documentation would improve maintainability and provide better IntelliSense support.

+/// <summary>
+/// Provides methods for managing JWT tokens in a Redis-backed store.
+/// </summary>
 public interface IRedisTokenStoreService
 {
+    /// <summary>
+    /// Stores a JWT token with associated email and timestamp in Redis.
+    /// </summary>
+    /// <param name="token">The JWT token to store.</param>
+    /// <param name="email">The email associated with the token.</param>
+    /// <param name="cancellationToken">Cancellation token for the operation.</param>
     Task StoreTokenAsync(string token, string email, CancellationToken cancellationToken = default);
+    
+    /// <summary>
+    /// Validates whether a token exists and is not revoked in the Redis store.
+    /// </summary>
+    /// <param name="token">The JWT token to validate.</param>
+    /// <param name="cancellationToken">Cancellation token for the operation.</param>
+    /// <returns>True if the token is valid; otherwise, false.</returns>
     Task<bool> IsTokenValidAsync(string token, CancellationToken cancellationToken = default);
+    
+    /// <summary>
+    /// Revokes a token by removing it from the Redis store.
+    /// </summary>
+    /// <param name="token">The JWT token to revoke.</param>
+    /// <param name="cancellationToken">Cancellation token for the operation.</param>
     Task RevokeTokenAsync(string token, CancellationToken cancellationToken = default);
 }
OsmoDoc/Word/Models/DocumentData.cs (1)

7-7: Remove extra blank line.

There's an unnecessary blank line here.

 namespace OsmoDoc.Word.Models;
 
-
 /// <summary>
OsmoDoc/Word/Models/ContentData.cs (1)

3-3: Remove unnecessary blank lines.

There are extra blank lines that should be removed for consistency.

 namespace OsmoDoc.Word.Models;
 
-
 /// <summary>
     /// Gets or sets the parent body of the placeholder (none or table).
     /// </summary>
-
     public ParentBody ParentBody { get; set; }

Also applies to: 27-27

OsmoDoc.API/Program.cs (2)

122-126: Simplify token extraction logic.

The token extraction can be simplified using string manipulation methods.

-if (context.Request.Headers.TryGetValue("Authorization", out Microsoft.Extensions.Primitives.StringValues authHeader) &&
-    authHeader.ToString().StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase))
-{
-    tokenString = authHeader.ToString().Substring("Bearer ".Length).Trim();
-}
+string? authHeader = context.Request.Headers.Authorization.FirstOrDefault();
+if (authHeader?.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase) == true)
+{
+    tokenString = authHeader["Bearer ".Length..].Trim();
+}

114-133: Consider caching token validation results.

Checking Redis on every request could impact performance. Consider implementing a short-lived cache for token validation results.

What are the best practices for caching JWT token validation results in Redis-backed authentication systems?
OsmoDoc.API/Controllers/WordController.cs (1)

47-56: Consider using IOptions pattern for configuration

Instead of retrieving configuration values in each request, consider injecting them using the IOptions pattern for better performance and testability.

OsmoDoc/Word/WordDocumentGenerator.cs (2)

173-173: Consider removing the unconditional spacing modification.

Setting SpacingAfter = 0 for all paragraphs containing placeholders may alter the original document formatting unexpectedly. This should either be configurable or removed.

If this is intentional behavior, consider making it configurable:

-            paragraph.SpacingAfter = 0;
+            // Only modify spacing if explicitly configured
+            // paragraph.SpacingAfter = 0;

262-262: Fix typo in documentation.

-    /// <param name="documentPath">The ile path where the updated document will be saved.</param>
+    /// <param name="documentPath">The file path where the updated document will be saved.</param>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d055cd3 and d565e15.

⛔ Files ignored due to path filters (1)
  • OsmoDoc.API/wwwroot/Tools/wkhtmltopdf.exe is excluded by !**/*.exe
📒 Files selected for processing (59)
  • .env.example (1 hunks)
  • .github/PULL_REQUEST_TEMPLATE/pull_request_template_api.md (1 hunks)
  • CONTRIBUTING.md (7 hunks)
  • Dockerfile (2 hunks)
  • DocumentService.API/Helpers/AutoMappingProfile.cs (0 hunks)
  • DocumentService.API/Models/PdfGenerationRequestDTO.cs (0 hunks)
  • DocumentService.API/Models/WordGenerationRequestDTO.cs (0 hunks)
  • DocumentService/DocumentService.csproj (0 hunks)
  • DocumentService/Pdf/Models/ContentMetaData.cs (0 hunks)
  • DocumentService/Pdf/Models/DocumentData.cs (0 hunks)
  • DocumentService/Pdf/PdfDocumentGenerator.cs (0 hunks)
  • DocumentService/Word/Models/ContentData.cs (0 hunks)
  • DocumentService/Word/Models/DocumentData.cs (0 hunks)
  • DocumentService/Word/Models/Enums.cs (0 hunks)
  • DocumentService/Word/Models/TableData.cs (0 hunks)
  • DocumentService/Word/WordDocumentGenerator.cs (0 hunks)
  • OsmoDoc.API/Controllers/LoginController.cs (1 hunks)
  • OsmoDoc.API/Controllers/PdfController.cs (1 hunks)
  • OsmoDoc.API/Controllers/WordController.cs (1 hunks)
  • OsmoDoc.API/DotEnv.cs (1 hunks)
  • OsmoDoc.API/Helpers/AuthenticationHelper.cs (1 hunks)
  • OsmoDoc.API/Helpers/AutoMappingProfile.cs (1 hunks)
  • OsmoDoc.API/Helpers/Base64StringHelper.cs (1 hunks)
  • OsmoDoc.API/Helpers/CommonMethodsHelper.cs (1 hunks)
  • OsmoDoc.API/Models/BaseResponse.cs (1 hunks)
  • OsmoDoc.API/Models/LoginRequestDTO.cs (1 hunks)
  • OsmoDoc.API/Models/PdfGenerationRequestDTO.cs (1 hunks)
  • OsmoDoc.API/Models/RevokeTokenRequestDTO.cs (1 hunks)
  • OsmoDoc.API/Models/WordGenerationRequestDTO.cs (1 hunks)
  • OsmoDoc.API/OsmoDoc.API.csproj (1 hunks)
  • OsmoDoc.API/OsmoDoc.API.sln (1 hunks)
  • OsmoDoc.API/Program.cs (1 hunks)
  • OsmoDoc.sln (1 hunks)
  • OsmoDoc/OsmoDoc.csproj (1 hunks)
  • OsmoDoc/Pdf/Models/ContentMetaData.cs (1 hunks)
  • OsmoDoc/Pdf/Models/DocumentData.cs (1 hunks)
  • OsmoDoc/Pdf/Models/OsmoDocPdfConfig.cs (1 hunks)
  • OsmoDoc/Pdf/PdfDocumentGenerator.cs (1 hunks)
  • OsmoDoc/Services/Interfaces/IRedisTokenStoreService.cs (1 hunks)
  • OsmoDoc/Services/RedisTokenStoreService.cs (1 hunks)
  • OsmoDoc/Word/Models/ContentData.cs (1 hunks)
  • OsmoDoc/Word/Models/DocumentData.cs (1 hunks)
  • OsmoDoc/Word/Models/Enums.cs (1 hunks)
  • OsmoDoc/Word/Models/ImageData.cs (1 hunks)
  • OsmoDoc/Word/Models/TableData.cs (1 hunks)
  • OsmoDoc/Word/WordDocumentGenerator.cs (1 hunks)
  • README.md (6 hunks)
  • docker-compose.yaml (1 hunks)
  • docs/site/10.0.2/api/OsmoDoc.Word.Models.ContentData.html (5 hunks)
  • docs/site/10.0.2/api/OsmoDoc.Word.Models.ContentType.html (2 hunks)
  • docs/site/10.0.2/api/OsmoDoc.Word.Models.DocumentData.html (4 hunks)
  • docs/site/10.0.2/api/OsmoDoc.Word.Models.ParentBody.html (2 hunks)
  • docs/site/10.0.2/api/OsmoDoc.Word.Models.TableData.html (3 hunks)
  • docs/site/10.0.2/api/OsmoDoc.Word.Models.html (2 hunks)
  • docs/site/10.0.2/api/OsmoDoc.Word.WordDocumentGenerator.html (3 hunks)
  • docs/site/10.0.2/api/OsmoDocWord.html (2 hunks)
  • docs/site/10.0.2/api/toc.html (1 hunks)
  • docs/site/manifest.json (1 hunks)
  • docs/site/xrefmap.yml (1 hunks)
💤 Files with no reviewable changes (12)
  • DocumentService/Pdf/Models/DocumentData.cs
  • DocumentService/Word/Models/ContentData.cs
  • DocumentService/Pdf/Models/ContentMetaData.cs
  • DocumentService/Word/Models/DocumentData.cs
  • DocumentService/Word/Models/TableData.cs
  • DocumentService.API/Helpers/AutoMappingProfile.cs
  • DocumentService/DocumentService.csproj
  • DocumentService.API/Models/PdfGenerationRequestDTO.cs
  • DocumentService/Word/Models/Enums.cs
  • DocumentService/Word/WordDocumentGenerator.cs
  • DocumentService.API/Models/WordGenerationRequestDTO.cs
  • DocumentService/Pdf/PdfDocumentGenerator.cs
🧰 Additional context used
🧠 Learnings (4)
OsmoDoc/Pdf/Models/DocumentData.cs (1)
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:46-47
Timestamp: 2025-06-21T16:48:17.092Z
Learning: In the OsmoDoc project, the DocumentData class properties Placeholders and TablesData are initialized with empty lists using `= new()`, which means they are never null and don't require null-coalescing operators when accessed.
OsmoDoc.API/Models/WordGenerationRequestDTO.cs (1)
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:32-47
Timestamp: 2025-06-21T17:00:19.917Z
Learning: In the OsmoDoc project, DocumentData class has its Placeholders and TablesData properties initialized to empty lists by default, and in WordGenerationRequestDTO, the DocumentData property is initialized with `= new();`. This pattern ensures that the properties are never null, though the parameter itself could still be null if explicitly passed as null.
OsmoDoc/Word/WordDocumentGenerator.cs (1)
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:32-47
Timestamp: 2025-06-21T17:00:19.917Z
Learning: In the OsmoDoc project, DocumentData class has its Placeholders and TablesData properties initialized to empty lists with `= new()`, and DTOs are also initialized properly. This pattern ensures that the properties are never null, making additional null checks unnecessary for these specific properties in the WordDocumentGenerator.GenerateDocumentByTemplate method.
OsmoDoc.API/Controllers/PdfController.cs (1)
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc.API/Controllers/PdfController.cs:34-46
Timestamp: 2025-06-18T14:34:35.114Z
Learning: In OsmoDoc.API/Controllers/PdfController.cs, the PdfDocumentGenerator.GeneratePdf method only deletes its internal temporary files (converted HTML, modified HTML, JSON data files) but does not delete the input template files or output PDF files created by the controller, requiring manual cleanup in the controller to prevent disk bloat.
🪛 dotenv-linter (3.3.0)
.env.example

[warning] 4-4: [UnorderedKey] The REDIS_PORT key should go before the REDIS_URL key

🪛 LanguageTool
README.md

[duplication] ~1-~1: Possible typo: you repeated a word.
Context: # OsmoDoc OsmoDoc is a library with the following functio...

(ENGLISH_WORD_REPEAT_RULE)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (csharp)
🔇 Additional comments (45)
.github/PULL_REQUEST_TEMPLATE/pull_request_template_api.md (1)

5-5: LGTM - URL updated for consistency.

The repository URL has been correctly updated to reflect the new "osmodoc" naming convention.

OsmoDoc.API/OsmoDoc.API.sln (1)

6-6: LGTM - Solution file correctly updated for project rename.

The project reference has been properly updated from "DocumentService.API" to "OsmoDoc.API" while maintaining the project GUID for continuity.

CONTRIBUTING.md (1)

1-2: LGTM - Contributing guidelines updated consistently.

All project references have been systematically updated from "document-service" to "osmodoc" including URLs, tags, and examples. The changes maintain consistency throughout the documentation.

Also applies to: 16-17, 26-27, 37-37, 43-43, 49-49, 66-66, 68-71, 93-93, 113-113

OsmoDoc.API/Helpers/Base64StringHelper.cs (1)

1-1: LGTM - Namespace updated for consistency.

The namespace has been correctly updated from DocumentService.API.Helpers to OsmoDoc.API.Helpers as part of the project-wide renaming effort.

OsmoDoc.API/OsmoDoc.API.csproj (2)

11-11: Aligned project with Redis dependencies

Adding StackExchange.Redis is necessary for the new Redis-based token store. Confirm that version 2.8.41 is compatible with .NET 8.0 or consider upgrading to the latest stable if there are known issues.


16-16: Updated project reference to OsmoDoc

The <ProjectReference> now points to OsmoDoc.csproj, reflecting the namespace rename. Ensure all other references to the old DocumentService project have been migrated similarly.

OsmoDoc.API/DotEnv.cs (1)

1-2: Namespace updated to OsmoDoc.API

The file-scoped namespace change aligns with the overall migration from DocumentService.API to OsmoDoc.API. No further action needed.

OsmoDoc/Pdf/Models/ContentMetaData.cs (1)

3-7: Simple data container is correct

ContentMetaData encapsulates a placeholder/content pair and defaults to non-null values. This matches its intended use in the PDF generator.

docs/site/10.0.2/api/OsmoDocWord.html (3)

8-12: All namespace references updated correctly.
The <title> and <meta name="title"> now consistently use OsmoDoc.Word. No leftover DocumentService mentions.


70-73: Content UID and heading reflect new namespace.
The data-uid attribute and <h1> text correctly reference OsmoDoc.Word.


80-81: Generator link updated.
The hyperlink to WordDocumentGenerator now points to OsmoDoc.Word.WordDocumentGenerator.html.

docs/site/10.0.2/api/OsmoDoc.Word.Models.html (3)

8-12: Head metadata reflects OsmoDoc.Word.Models.
Title and meta title updated correctly to the new namespace.


70-73: Article UID and heading updated.
The data-uid and <h1> now match OsmoDoc.Word.Models.


80-91: Model links correctly renamed.
All class and enum hyperlinks (ContentData, DocumentData, TableData, ContentType, ParentBody) now reference OsmoDoc.Word.Models.*.

docs/site/10.0.2/api/OsmoDoc.Word.Models.ContentType.html (3)

70-76: Enum UID and heading updated.
The data-uid and <h1> correctly reference OsmoDoc.Word.Models.ContentType.


79-80: Namespace and assembly declarations updated.
The <h6> tags correctly show OsmoDoc.Word.Models and OsmoDoc.dll.


96-102: Enum field IDs reflect new prefix.
Fields Image and Text have updated IDs OsmoDoc_Word_Models_ContentType_*.

docs/site/manifest.json (1)

8-81: ManagedReference entries updated to OsmoDoc.*.
All source_relative_path and relative_path fields now correctly reflect the OsmoDoc naming.

OsmoDoc.API/Helpers/AuthenticationHelper.cs (1)

6-6: Align namespace with project rename
The namespace has been correctly updated to OsmoDoc.API.Helpers to reflect the rebranding.

docs/site/10.0.2/api/OsmoDoc.Word.Models.ParentBody.html (1)

70-81: Documentation namespace rename is correct
All references to DocumentService.Word.Models.ParentBody have been updated to OsmoDoc.Word.Models.ParentBody consistently.

docs/site/10.0.2/api/OsmoDoc.Word.Models.ContentData.html (1)

70-78: Documentation namespace update
The data-uid, heading IDs, and namespace/assembly references have been correctly adjusted to OsmoDoc.Word.Models.ContentData with no lingering legacy names.

Also applies to: 107-108

README.md (2)

1-2: Project header updated to OsmoDoc
The main title has been renamed from the old name to OsmoDoc appropriately.


193-195: Contributor badge is correct
The contributors badge and link now point to the osmodoc repository as intended.

docs/site/10.0.2/api/OsmoDoc.Word.Models.DocumentData.html (1)

70-156: Namespace updates are consistent throughout the documentation.

The HTML documentation has been properly updated to reflect the namespace change from DocumentService.Word.Models to OsmoDoc.Word.Models. All UIDs, data-uid attributes, and cross-references are consistently updated.

docs/site/10.0.2/api/OsmoDoc.Word.Models.TableData.html (1)

70-161: Documentation namespace updates are properly applied.

All namespace references, UIDs, and cross-references have been consistently updated from DocumentService.Word.Models to OsmoDoc.Word.Models.

docs/site/xrefmap.yml (2)

4-195: Cross-reference mappings consistently updated for namespace migration.

All UIDs, names, href links, and comment IDs have been systematically updated from DocumentService.Word to OsmoDoc.Word namespace. The changes are comprehensive and consistent.


1-195: Inconsistency between PR objectives and actual changes.

The PR objectives mention adding "feature for handling multiple image source types" and introducing an "ImageData class," but the provided files show:

  1. Namespace refactoring from DocumentService to OsmoDoc
  2. New token revocation functionality
  3. PDF configuration updates
  4. Documentation updates

No ImageData class or image handling features are present in the reviewed files.

Please verify if:

  1. The correct files were provided for review
  2. The PR description accurately reflects the changes
  3. The image handling features are in additional files not included in this review
#!/bin/bash
# Search for ImageData class and image-related functionality
echo "Searching for ImageData class..."
fd -e cs | xargs rg -l "ImageData"

echo "Searching for image-related functionality..."
rg -i "image.*source|image.*type|image.*handling" --type cs

Likely an incorrect or invalid review comment.

OsmoDoc/OsmoDoc.csproj (2)

1-28: Inconsistency between PR objectives and actual changes.

The PR objectives state this is about "add feature for handling multiple image source types," but the actual changes show a comprehensive project rename from "DocumentService" to "OsmoDoc" with authentication infrastructure additions. Please ensure the PR description accurately reflects the actual changes.

Likely an incorrect or invalid review comment.


22-25: Verify package versions for security and compatibility.

Please verify that these package versions are current and free from known vulnerabilities:

  • DocumentFormat.OpenXml 2.19.0
  • Newtonsoft.Json 13.0.3
  • NPOI 2.7.2
  • StackExchange.Redis 2.8.41
Are there any known security vulnerabilities in DocumentFormat.OpenXml version 2.19.0, Newtonsoft.Json version 13.0.3, NPOI version 2.7.2, or StackExchange.Redis version 2.8.41?
OsmoDoc.sln (1)

6-8: Project renaming looks correct.

The solution file properly reflects the project rename from "DocumentService" to "OsmoDoc" while preserving the project GUIDs, which is the correct approach for maintaining existing references and configurations.

OsmoDoc.API/Helpers/CommonMethodsHelper.cs (2)

9-12: Good addition of input validation.

The input validation for filePath is a solid improvement that prevents potential null reference exceptions and provides clear error messages.


16-16: Correct nullable annotation.

Making directoryName nullable is correct since Path.GetDirectoryName() can return null for root paths or invalid inputs.

docs/site/10.0.2/api/toc.html (1)

17-44: Documentation updates correctly reflect namespace changes.

All references have been consistently updated from "DocumentService.Word" to "OsmoDoc.Word" throughout the table of contents, maintaining proper HTML structure and link integrity.

OsmoDoc/Word/Models/TableData.cs (1)

1-22: LGTM! Clean and well-documented model class.

The TableData class is well-structured with appropriate XML documentation and follows C# conventions. The initialization of the Data property to an empty list prevents null reference exceptions.

OsmoDoc/Word/Models/Enums.cs (1)

7-13: Inconsistency with PR objectives regarding image handling.

The PR objectives state this is about "add feature for handling multiple image source types," but the ContentType enum only contains Text = 0. According to the AI summary, an Image member was actually removed from the previous version. This seems contradictory to the stated goal of enhancing image handling capabilities.

Please verify if image handling is implemented elsewhere or if the PR objectives need to be updated to reflect the actual changes.

#!/bin/bash
# Search for image-related classes and enums in the codebase
echo "Searching for ImageData class mentioned in PR objectives..."
fd -t f -e cs | xargs rg -l "ImageData"

echo "Searching for image-related enums or types..."
fd -t f -e cs | xargs rg -A 5 -B 5 "Image.*="

echo "Searching for image handling methods..."
fd -t f -e cs | xargs rg -A 5 -B 5 "image|Image" --type cs

Likely an incorrect or invalid review comment.

docs/site/10.0.2/api/OsmoDoc.Word.WordDocumentGenerator.html (1)

70-139: Documentation updates are consistent with namespace refactoring.

The HTML documentation has been properly updated to reflect the namespace changes from DocumentService.Word to OsmoDoc.Word. All references including data-uid attributes, class IDs, namespace links, and parameter type references have been consistently updated.

docker-compose.yaml (2)

10-24: Service renaming is consistent with project refactoring.

The service name, image, and container name updates align well with the overall transition from DocumentService to OsmoDoc.


2-9: Redis service configuration looks good.

The Redis service is properly configured with the official Redis 7 image and environment-based port configuration.

Please verify that the required Redis environment variables are documented in .env.example:

#!/bin/bash
# Check if Redis environment variables are documented
echo "Checking .env.example for Redis configuration..."
cat .env.example | grep -i redis || echo "No Redis configuration found in .env.example"

echo "Checking for Redis-related environment variables..."
rg "REDIS_" --type env || echo "No REDIS_ environment variables found"
OsmoDoc.API/Models/PdfGenerationRequestDTO.cs (1)

6-12: Well-structured DTO with proper validation.

The PdfGenerationRequestDTO class follows good practices with appropriate validation attributes and property initialization. The design supports both direct PDF generation from base64 templates and EJS template processing.

OsmoDoc/Word/Models/DocumentData.cs (1)

11-28: Well-structured document data model.

The class properly encapsulates document elements with initialized collections preventing null reference issues. The addition of the Images property aligns with the PR objective for handling multiple image source types.

OsmoDoc.API/Models/BaseResponse.cs (1)

24-38: Excellent refactoring of model validation error handling.

The improved implementation properly handles null ModelState values and provides a clear default message. The use of LINQ with null-safe navigation and filtering ensures robust error extraction.

OsmoDoc/Word/Models/ContentData.cs (1)

19-23: Let’s pull the enum definition to confirm its values:

#!/bin/bash
# Show the ContentType enum block with context
rg -n -A20 "public enum ContentType" OsmoDoc/Word/Models/Enums.cs
OsmoDoc.API/Program.cs (1)

29-30: ```shell
#!/bin/bash

Locate the DotEnv class implementation

rg -l "class DotEnv" -t cs

Display the DotEnv class to inspect Load method behavior

rg -A 50 "class DotEnv" -t cs


</details>
<details>
<summary>OsmoDoc.API/Models/WordGenerationRequestDTO.cs (1)</summary>

`1-21`: **Well-structured DTOs with proper null-safety**

The DTOs follow best practices with proper initialization and validation attributes, ensuring null-safety as per the project's patterns.

</details>
<details>
<summary>OsmoDoc.API/Controllers/PdfController.cs (1)</summary>

`27-124`: Let’s correct the search and rerun without the invalid `-R` flag to locate any cleanup helpers or `File.Delete` usage:


```shell
#!/bin/bash
# Search for custom delete/cleanup helper methods
rg -t cs "public static .*Delete.*\(" .
rg -t cs "public static .*Clean.*\(" .

# Search for direct File.Delete calls
rg -t cs "File.Delete" .

Copy link

@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

🧹 Nitpick comments (1)
OsmoDoc/Word/WordDocumentGenerator.cs (1)

267-267: Fix typo in XML documentation.

There's a typo in the XML comment: "ile path" should be "file path".

-    /// <param name="documentPath">The ile path where the updated document will be saved.</param>
+    /// <param name="documentPath">The file path where the updated document will be saved.</param>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f6650e1 and b0ad8b8.

📒 Files selected for processing (1)
  • OsmoDoc/Word/WordDocumentGenerator.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc.API/Controllers/WordController.cs:38-60
Timestamp: 2025-06-18T15:05:30.284Z
Learning: osm-Jatin prefers to create separate GitHub issues to track technical debt and improvement suggestions that are out of scope for the current PR, such as temporary file cleanup concerns in the OsmoDoc project.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#41
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:10-27
Timestamp: 2025-06-19T04:10:32.232Z
Learning: In the OsmoDoc project, they use a hybrid JWT + Redis token management approach: non-expiry JWT tokens are generated and stored in Redis for validation and manual revocation. The system includes RedisTokenService for token storage/validation and dedicated revocation endpoints, allowing stateless JWT benefits while maintaining token lifecycle control.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#42
File: OsmoDoc/Services/RedisTokenStoreService.cs:18-24
Timestamp: 2025-06-20T14:36:49.953Z
Learning: In the OsmoDoc project, the team prefers manual token lifecycle management over automatic TTL expiration in Redis. They deliberately store tokens without expiration to maintain precise control over when tokens become invalid through their dedicated revocation endpoints, rather than relying on automatic expiration.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#42
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:18-24
Timestamp: 2025-06-20T14:37:36.118Z
Learning: In OsmoDoc.API, JWT tokens are intentionally created without expiration because the application uses Redis as the authoritative source for token validity. Tokens are stored in Redis when issued and can be manually revoked by removing them from Redis, providing centralized token management and immediate invalidation capability.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#49
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:11-24
Timestamp: 2025-06-22T13:55:51.706Z
Learning: In the OsmoDoc project, JWT tokens are intentionally created without expiration dates. The team has chosen to implement manual token invalidation through Redis storage instead of using JWT expiration claims. Tokens are stored in Redis and can be manually revoked using the /api/revoke endpoint. This design decision was made in PR #42 where the user stated "Not needed, we will make the token inactive manually for now."
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#41
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:10-27
Timestamp: 2025-06-19T04:10:32.232Z
Learning: In the OsmoDoc project, non-expiry JWT tokens are used intentionally with manual invalidation mechanisms. The team handles token lifecycle management manually rather than using time-based expiration.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#42
File: OsmoDoc.API/Program.cs:98-112
Timestamp: 2025-06-20T14:36:43.563Z
Learning: In the OsmoDoc project, JWT tokens are intentionally designed to be non-expiring. The custom LifetimeValidator in Program.cs always returns true to prevent token expiration validation, which is by design rather than a bug.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Pdf/PdfDocumentGenerator.cs:84-92
Timestamp: 2025-06-18T14:44:46.570Z
Learning: osm-Jatin prefers to document input requirements clearly in the README file rather than adding runtime validation code for scenarios like null/duplicate placeholders in the OsmoDoc PDF generation functionality.
OsmoDoc/Word/WordDocumentGenerator.cs (13)
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:32-47
Timestamp: 2025-06-21T17:00:19.937Z
Learning: In the OsmoDoc project, DocumentData class has its Placeholders and TablesData properties initialized to empty lists with `= new()`, and DTOs are also initialized properly. This pattern ensures that the properties are never null, making additional null checks unnecessary for these specific properties in the WordDocumentGenerator.GenerateDocumentByTemplate method.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:32-47
Timestamp: 2025-06-21T17:00:19.937Z
Learning: In the OsmoDoc project, DocumentData class has its Placeholders and TablesData properties initialized to empty lists by default, and in WordGenerationRequestDTO, the DocumentData property is initialized with `= new();`. This pattern ensures that the properties are never null, though the parameter itself could still be null if explicitly passed as null.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:46-47
Timestamp: 2025-06-21T16:48:17.115Z
Learning: In the OsmoDoc project, the DocumentData class properties Placeholders and TablesData are initialized with empty lists using `= new()`, which means they are never null and don't require null-coalescing operators when accessed.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#49
File: OsmoDoc/Word/WordDocumentGenerator.cs:33-44
Timestamp: 2025-07-04T03:15:02.254Z
Learning: In the OsmoDoc project, empty DocumentData collections (Placeholders, TablesData, Images) are valid scenarios when users want to generate documents without any placeholder replacements, table data, or images. The DocumentData parameter is always initialized at the controller level, but null checks are still added for defensive programming.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#42
File: OsmoDoc/Word/WordDocumentGenerator.cs:0-0
Timestamp: 2025-06-20T14:47:35.545Z
Learning: NPOI's XWPFDocument.Write method does not support async operations (WriteAsync is not available), so when working with NPOI Word document generation, synchronous write operations should be used rather than attempting to make them async.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc.API/Controllers/PdfController.cs:34-46
Timestamp: 2025-06-18T14:34:35.161Z
Learning: In OsmoDoc.API/Controllers/PdfController.cs, the PdfDocumentGenerator.GeneratePdf method only deletes its internal temporary files (converted HTML, modified HTML, JSON data files) but does not delete the input template files or output PDF files created by the controller, requiring manual cleanup in the controller to prevent disk bloat.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:298-311
Timestamp: 2025-06-21T17:11:15.672Z
Learning: In the OsmoDoc Word document generation code, when checking mainDocumentPart for null before populating a drawings collection, an additional null check inside the foreach loop is redundant because if mainDocumentPart was null, the drawings collection would remain empty and the loop wouldn't execute.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Pdf/PdfDocumentGenerator.cs:24-37
Timestamp: 2025-06-18T14:03:39.731Z
Learning: In OsmoDoc PdfDocumentGenerator.GeneratePdf method, an empty metaDataList is a valid scenario when users want to convert HTML/EJS templates to PDF without any placeholder replacements. Only null metaDataList should be validated, not empty ones.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:298-311
Timestamp: 2025-06-21T17:11:15.672Z
Learning: When using null-conditional operators like `docProperty?.Name != null` in C#, the compiler may still require null-forgiving operators (`!`) in subsequent usage within the same conditional block to suppress warnings, even when the null check logically ensures the value is non-null.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Pdf/PdfDocumentGenerator.cs:84-92
Timestamp: 2025-06-18T14:44:46.570Z
Learning: osm-Jatin prefers to document input requirements clearly in the README file rather than adding runtime validation code for scenarios like null/duplicate placeholders in the OsmoDoc PDF generation functionality.
🪛 GitHub Actions: PR #49
OsmoDoc/Word/WordDocumentGenerator.cs

[error] 384-384: CS0152: The switch statement contains multiple cases with the label value '1'.

🔇 Additional comments (4)
OsmoDoc/Word/WordDocumentGenerator.cs (4)

40-43: Null check added as requested.

The null check for documentData has been properly added following the previous review discussion. This defensive programming approach is good practice even though the parameter is initialized at the controller level.


385-389: HttpClient usage follows established project pattern.

The HttpClient instantiation within the using block follows the established pattern preferred by the team, as noted in previous reviews for this project.


270-276: Proper validation for empty image collections.

The method correctly handles the case where images collection is null or empty, which aligns with the project's approach of supporting empty collections as valid scenarios.


347-356: Robust cleanup with appropriate error handling.

The cleanup logic in the finally block properly handles potential errors when deleting temporary files. Ignoring cleanup errors is appropriate here since the main operation has already completed successfully.

Copy link

@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: 6

🧹 Nitpick comments (2)
OsmoDoc/Pdf/PdfDocumentGenerator.cs (2)

53-56: Use Path.GetExtension for case-insensitive comparison.

The current implementation converts to lowercase for comparison, but you can use StringComparison.OrdinalIgnoreCase for more explicit case-insensitive comparison.

-if (Path.GetExtension(templatePath).ToLower() != ".ejs")
+if (!Path.GetExtension(templatePath).Equals(".ejs", StringComparison.OrdinalIgnoreCase))

249-263: Add support for macOS platform.

The method only handles Windows and Linux, but macOS support might be needed for development environments.

 private static string EjsToHtmlArgumentsBasedOnOS(string ejsFilePath, string ejsDataJsonFilePath, string tempHtmlFilePath)
 {
     if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
     {
         return $"/C ejs \"{ejsFilePath}\" -f \"{ejsDataJsonFilePath}\" -o \"{tempHtmlFilePath}\"";
     }
-    else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
+    else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
     {
         return $"\"{ejsFilePath}\" -f \"{ejsDataJsonFilePath}\" -o \"{tempHtmlFilePath}\"";
     }
     else
     {
         throw new Exception("Unknown operating system");
     }
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0ad8b8 and 411bc5f.

📒 Files selected for processing (2)
  • OsmoDoc/Pdf/PdfDocumentGenerator.cs (1 hunks)
  • OsmoDoc/Word/Models/ImageData.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • OsmoDoc/Word/Models/ImageData.cs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc.API/Controllers/WordController.cs:38-60
Timestamp: 2025-06-18T15:05:30.284Z
Learning: osm-Jatin prefers to create separate GitHub issues to track technical debt and improvement suggestions that are out of scope for the current PR, such as temporary file cleanup concerns in the OsmoDoc project.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#41
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:10-27
Timestamp: 2025-06-19T04:10:32.232Z
Learning: In the OsmoDoc project, they use a hybrid JWT + Redis token management approach: non-expiry JWT tokens are generated and stored in Redis for validation and manual revocation. The system includes RedisTokenService for token storage/validation and dedicated revocation endpoints, allowing stateless JWT benefits while maintaining token lifecycle control.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#42
File: OsmoDoc/Services/RedisTokenStoreService.cs:18-24
Timestamp: 2025-06-20T14:36:49.953Z
Learning: In the OsmoDoc project, the team prefers manual token lifecycle management over automatic TTL expiration in Redis. They deliberately store tokens without expiration to maintain precise control over when tokens become invalid through their dedicated revocation endpoints, rather than relying on automatic expiration.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#42
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:18-24
Timestamp: 2025-06-20T14:37:36.118Z
Learning: In OsmoDoc.API, JWT tokens are intentionally created without expiration because the application uses Redis as the authoritative source for token validity. Tokens are stored in Redis when issued and can be manually revoked by removing them from Redis, providing centralized token management and immediate invalidation capability.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#49
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:11-24
Timestamp: 2025-06-22T13:55:51.706Z
Learning: In the OsmoDoc project, JWT tokens are intentionally created without expiration dates. The team has chosen to implement manual token invalidation through Redis storage instead of using JWT expiration claims. Tokens are stored in Redis and can be manually revoked using the /api/revoke endpoint. This design decision was made in PR #42 where the user stated "Not needed, we will make the token inactive manually for now."
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#41
File: OsmoDoc.API/Helpers/AuthenticationHelper.cs:10-27
Timestamp: 2025-06-19T04:10:32.232Z
Learning: In the OsmoDoc project, non-expiry JWT tokens are used intentionally with manual invalidation mechanisms. The team handles token lifecycle management manually rather than using time-based expiration.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#42
File: OsmoDoc.API/Program.cs:98-112
Timestamp: 2025-06-20T14:36:43.563Z
Learning: In the OsmoDoc project, JWT tokens are intentionally designed to be non-expiring. The custom LifetimeValidator in Program.cs always returns true to prevent token expiration validation, which is by design rather than a bug.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Pdf/PdfDocumentGenerator.cs:84-92
Timestamp: 2025-06-18T14:44:46.570Z
Learning: osm-Jatin prefers to document input requirements clearly in the README file rather than adding runtime validation code for scenarios like null/duplicate placeholders in the OsmoDoc PDF generation functionality.
OsmoDoc/Pdf/PdfDocumentGenerator.cs (8)
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc.API/Controllers/PdfController.cs:34-46
Timestamp: 2025-06-18T14:34:35.161Z
Learning: In OsmoDoc.API/Controllers/PdfController.cs, the PdfDocumentGenerator.GeneratePdf method only deletes its internal temporary files (converted HTML, modified HTML, JSON data files) but does not delete the input template files or output PDF files created by the controller, requiring manual cleanup in the controller to prevent disk bloat.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Pdf/PdfDocumentGenerator.cs:24-37
Timestamp: 2025-06-18T14:03:39.731Z
Learning: In OsmoDoc PdfDocumentGenerator.GeneratePdf method, an empty metaDataList is a valid scenario when users want to convert HTML/EJS templates to PDF without any placeholder replacements. Only null metaDataList should be validated, not empty ones.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#43
File: OsmoDoc/Word/WordDocumentGenerator.cs:32-47
Timestamp: 2025-06-21T17:00:19.937Z
Learning: In the OsmoDoc project, DocumentData class has its Placeholders and TablesData properties initialized to empty lists with `= new()`, and DTOs are also initialized properly. This pattern ensures that the properties are never null, making additional null checks unnecessary for these specific properties in the WordDocumentGenerator.GenerateDocumentByTemplate method.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Word/WordDocumentGenerator.cs:314-315
Timestamp: 2025-06-19T18:07:02.969Z
Learning: In the OsmoDoc/Word/WordDocumentGenerator.cs file, the team prefers to keep the HttpClient usage as it is currently implemented (creating a new HttpClient instance for each image download), despite potential performance considerations. This decision was made with reference to discussion in PR #41.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc.API/Controllers/WordController.cs:38-60
Timestamp: 2025-06-18T15:05:30.284Z
Learning: osm-Jatin prefers to create separate GitHub issues to track technical debt and improvement suggestions that are out of scope for the current PR, such as temporary file cleanup concerns in the OsmoDoc project.
Learnt from: osm-Jatin
PR: OsmosysSoftware/osmodoc#40
File: OsmoDoc/Pdf/PdfDocumentGenerator.cs:84-92
Timestamp: 2025-06-18T14:44:46.570Z
Learning: osm-Jatin prefers to document input requirements clearly in the README file rather than adding runtime validation code for scenarios like null/duplicate placeholders in the OsmoDoc PDF generation functionality.

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.

2 participants