feat: add redis database to store the jwt tokens #42
Conversation
… all related namespaces
WalkthroughThis update rebrands the project from "DocumentService" to "OsmoDoc," renaming namespaces, files, and documentation throughout. It introduces Redis-backed JWT token management, adds new API endpoints for login and token revocation, and implements asynchronous PDF and Word document generation using updated models and service interfaces. Docker and environment configurations are revised for Redis integration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as OsmoDoc.API
participant Redis
participant PDFGen as PdfDocumentGenerator
participant WordGen as WordDocumentGenerator
Client->>API: POST /api/login (email)
API->>API: Generate JWT token
API->>Redis: Store token with email
API-->>Client: BaseResponse (token)
Client->>API: POST /api/revoke (token)
API->>Redis: Revoke token
API-->>Client: BaseResponse (revoked)
Client->>API: POST /api/pdf (PDF generation request)
API->>PDFGen: GeneratePdf (async)
PDFGen->>PDFGen: (If EJS) Convert EJS to HTML
PDFGen->>PDFGen: Replace placeholders
PDFGen->>PDFGen: Convert HTML to PDF
PDFGen-->>API: PDF file path
API-->>Client: BaseResponse (PDF)
Client->>API: POST /api/word (Word generation request)
API->>WordGen: GenerateDocumentByTemplate (async)
WordGen->>WordGen: Replace text/table/image placeholders
WordGen-->>API: Word file path
API-->>Client: BaseResponse (Word file)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (29)
.env.example (2)
1-1: Avoid gitleaks false positive on placeholder
TheJWT_KEYplaceholder may trigger Gitleaks as a generic API key. Consider using a template comment or adding a comment to skip leak scanning to avoid false positives.
3-4: Order environment variables for consistency
According to dotenv-linter,REDIS_PORTshould be listed beforeREDIS_URL. Swap their order to satisfy the linter and maintain alphabetical consistency.CONTRIBUTING.md (1)
68-70: Fix malformed bullet points and remove HTML
The inline HTML and replacement-character bullets (�) break Markdown rendering. Replace these with standard Markdown list syntax (-or*) and remove the<strong>tag for clarity.OsmoDoc/Pdf/Models/ContentMetaData.cs (1)
5-6: Initialize non-nullable properties or mark as nullable/required
The auto-propertiesPlaceholderandContentare non-nullable but lack initialization. Consider addingrequiredmodifiers or default values (e.g.,= string.Empty;) to satisfy nullable-reference checks.OsmoDoc/Pdf/Models/DocumentData.cs (1)
1-3: Remove unused using directive & improve type abstractionThe
using System.Text;directive is unused and can be removed. Consider exposing thePlaceholdersproperty asIList<ContentMetaData>orIEnumerable<ContentMetaData>to depend on the interface rather than the concreteList<T>type.- using System.Text; namespace OsmoDoc.Pdf.Models; public class DocumentData { - public List<ContentMetaData> Placeholders { get; set; } = new List<ContentMetaData>(); + public IList<ContentMetaData> Placeholders { get; set; } = new List<ContentMetaData>(); }Also applies to: 5-9
OsmoDoc.API/Helpers/AuthenticationHelper.cs (1)
11-12: Parameter naming should follow camelCase.
LoginEmailshould beloginEmailto match C# parameter conventions.OsmoDoc.API/Models/BaseResponse.cs (2)
15-15: MakeStatusnon-nullable.
Statusis always set via the constructor; remove the nullable operator to reflect this invariant.
24-32: Handle empty ModelState gracefully.
FirstOrDefault()may return null, causingBadRequestObjectResult(null). Consider returning a default error response when no specific model errors exist.README.md (3)
1-2: Remove duplicated project name.The heading and the first sentence both start with "OsmoDoc"; consider rephrasing line 2 to avoid repetition.
27-27: Clarify repository URL in clone step.Specify the full Git clone URL (e.g.,
git clone https://github.com/OsmosysSoftware/osmodoc.git) to avoid ambiguity.
123-127: Avoid hard-coded absolute file paths in examples.The Windows example uses local absolute paths; switch to relative or generic placeholders to improve portability.
OsmoDoc.API/Models/LoginRequestDTO.cs (1)
7-9: Preferrequired+ init-only property for stronger null-safetySince the DTO is meant for inbound requests and
string.Emptyworkaround and leverages the C# 11requiredkeyword.- public string Email { get; set; } = string.Empty; + public required string Email { get; init; }OsmoDoc/OsmoDoc.csproj (2)
4-4: Use the singular<TargetFramework>element for a single TFM
<TargetFrameworks>is intended for multi-targeting. When you have only one framework it is cleaner (and avoids accidental tooling confusion) to use<TargetFramework>.- <TargetFrameworks>net8.0</TargetFrameworks> + <TargetFramework>net8.0</TargetFramework>
16-16: Replace placeholderPackageProjectUrlbefore publishing
URL to project homepage or documentationis still a placeholder. Either supply the correct URL or drop the property to avoid shipping invalid NuGet metadata.OsmoDoc.API/Models/RevokeTokenRequestDTO.cs (1)
7-8: MakeTokena required init-only stringSame rationale as for
LoginRequestDTO: the property is mandatory and should be immutable after model-binding.- public string Token { get; set; } = string.Empty; + public required string Token { get; init; }OsmoDoc/Word/Models/DocumentData.cs (1)
1-3: Remove unused using directives
SystemandSystem.Textare not referenced in this file.-using System; -using System.Text;OsmoDoc.API/Controllers/WordController.cs (1)
63-99: Minor: early-exit if no placeholders.
foreachruns even whenPlaceholdersis empty – harmless but we can skip the overhead:- // Handle image placeholder data in request - foreach (WordContentDataRequestDTO placeholder in request.DocumentData.Placeholders) + // Handle image placeholder data in request + if (request.DocumentData.Placeholders?.Count > 0) + foreach (WordContentDataRequestDTO placeholder in request.DocumentData.Placeholders)Purely optional; feel free to ignore.
docker-compose.yaml (2)
2-8: Persist and health-check RedisConsider adding a volume and a simple health-check so tokens survive container restarts and the API waits until Redis is ready:
redis: image: redis:7 container_name: redis + volumes: + - redis-data:/data + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 10s + retries: 5
10-16: Declare dependency ordering
osmodocneeds Redis before it can start; adddepends_onto avoid boot-time connection failures.osmodoc: ... + depends_on: + - redisOsmoDoc.API/Program.cs (2)
32-35: Non-blocking Redis connect
ConnectionMultiplexer.Connectis synchronous and blocks startup; preferConnectAsyncwithAbortOnConnectFail=falseto let the API boot even if Redis needs a few seconds.builder.Services.AddSingleton<IConnectionMultiplexer>(sp => ConnectionMultiplexer.Connect(new ConfigurationOptions { EndPoints = { Environment.GetEnvironmentVariable("REDIS_URL")! }, AbortOnConnectFail = false }));
38-50: Null / parsing guard for request-size config
Convert.ToInt64(builder.Configuration.GetSection("CONFIG:REQUEST_BODY_SIZE_LIMIT_BYTES").Value)throws if the key is missing or non-numeric. UseTryParsewith a sensible default.OsmoDoc/Services/RedisTokenStoreService.cs (1)
20-23: Unnecessary JSON payloadOnly key existence is ever checked; the serialized JSON is unused. Consider storing an empty string or the expiry timestamp to save memory and avoid Newtonsoft dependency.
OsmoDoc.API/Controllers/LoginController.cs (2)
12-18: Typo in private field
_tokenStoreSerivce→_tokenStoreService. Keeps codebase consistent.
28-30: Avoid unnecessary Task.Run
AuthenticationHelper.JwtTokenGeneratoris synchronous; wrapping it inTask.Runadds thread-pool overhead. Call it directly.OsmoDoc/Pdf/PdfDocumentGenerator.cs (1)
136-144: Read streams before awaiting exit to avoid deadlockRedirected output/error buffers can fill and block
WaitForExitAsync. Start reading the streams before awaiting exit or useprocess.BeginOutputReadLine.OsmoDoc/Word/WordDocumentGenerator.cs (4)
31-31: Fix method modifier order to follow C# conventions.- public async static Task GenerateDocumentByTemplate(string templateFilePath, DocumentData documentData, string outputFilePath) + public static async Task GenerateDocumentByTemplate(string templateFilePath, DocumentData documentData, string outputFilePath)
74-74: Enhance regex pattern to support more placeholder formats.The current regex pattern
@"{[a-zA-Z]+}"only matches alphabetic characters. Consider supporting numbers, underscores, and hyphens for more flexible placeholder names.- if (paragraph.ParagraphText == string.Empty || !new Regex(@"{[a-zA-Z]+}").IsMatch(paragraph.ParagraphText)) + if (paragraph.ParagraphText == string.Empty || !new Regex(@"{[a-zA-Z0-9_-]+}").IsMatch(paragraph.ParagraphText))Apply the same pattern update to lines 162 and 197.
Also applies to: 162-162, 197-197
114-117: Remove unnecessary try-catch block or add meaningful error handling.The current exception handling adds no value. Either remove the try-catch or add logging/context.
Remove the try-catch block entirely:
- try - { List<ContentData> contentData = documentData.Placeholders; // ... rest of the method - } - catch (Exception) - { - throw; - }
175-175: Avoid modifying paragraph spacing unconditionally.Setting
SpacingAfter = 0alters the original document formatting, which might not be intended.Consider removing this line or making it configurable:
- paragraph.SpacingAfter = 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
OsmoDoc.API/wwwroot/Tools/wkhtmltopdf.exeis excluded by!**/*.exe
📒 Files selected for processing (55)
.env.example(1 hunks).github/PULL_REQUEST_TEMPLATE/pull_request_template_api.md(1 hunks)CONTRIBUTING.md(7 hunks)Dockerfile(2 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/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 (9)
- DocumentService/Pdf/Models/DocumentData.cs
- DocumentService/Word/Models/TableData.cs
- DocumentService/Word/Models/DocumentData.cs
- DocumentService/DocumentService.csproj
- DocumentService/Word/Models/ContentData.cs
- DocumentService/Word/Models/Enums.cs
- DocumentService/Pdf/Models/ContentMetaData.cs
- DocumentService/Pdf/PdfDocumentGenerator.cs
- DocumentService/Word/WordDocumentGenerator.cs
🧰 Additional context used
🪛 GitHub Check: build
OsmoDoc/Word/Models/TableData.cs
[warning] 21-21:
Non-nullable property 'Data' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
OsmoDoc/Pdf/Models/ContentMetaData.cs
[warning] 6-6:
Non-nullable property 'Content' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
[warning] 5-5:
Non-nullable property 'Placeholder' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
OsmoDoc/Word/Models/ContentData.cs
[warning] 17-17:
Non-nullable property 'Content' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
[warning] 12-12:
Non-nullable property 'Placeholder' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
OsmoDoc/Word/Models/DocumentData.cs
[warning] 22-22:
Non-nullable property 'TablesData' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
[warning] 16-16:
Non-nullable property 'Placeholders' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
OsmoDoc/Word/WordDocumentGenerator.cs
[warning] 300-300:
Possible null reference argument for parameter 'id' in 'OpenXmlPart OpenXmlPartContainer.GetPartById(string id)'.
[warning] 300-300:
Dereference of a possibly null reference.
[warning] 294-294:
Possible null reference argument for parameter 'key' in 'bool Dictionary<string, string>.ContainsKey(string key)'.
🪛 Gitleaks (8.26.0)
.env.example
1-1: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 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 (36)
.github/PULL_REQUEST_TEMPLATE/pull_request_template_api.md (1)
5-5: Updated PR template link
The link to the CONTRIBUTING guidelines has been correctly updated to point at theosmodocrepository.OsmoDoc/Pdf/Models/OsmoDocPdfConfig.cs (1)
1-7: PDF config placeholder is appropriate
The newOsmoDocPdfConfigstatic class cleanly centralizes thewkhtmltopdfpath configuration. No issues found.CONTRIBUTING.md (1)
16-17: All project-name references updated correctly
All occurrences ofdocument-servicehave been replaced withosmodoc, and all URLs now point to the correctOsmosysSoftware/osmodocrepository.Also applies to: 26-27, 43-43, 49-49, 66-66, 93-93, 113-113
OsmoDoc.API/Helpers/Base64StringHelper.cs (1)
1-1: Namespace updated correctly
The namespace has been changed from the oldDocumentService.API.HelperstoOsmoDoc.API.Helpersas expected. No logic changes needed.OsmoDoc.API/DotEnv.cs (1)
1-1: Namespace update is consistent
The namespace has been correctly renamed toOsmoDoc.API, aligning with the project rebranding.OsmoDoc.API/OsmoDoc.API.csproj (2)
11-11: Redis dependency added
AddingStackExchange.Redis v2.8.41enables JWT token storage in Redis. Ensure this version is compatible with .NET 8.0 and aligns with your security policy.
16-16: Project reference updated
The project reference now points to..\OsmoDoc\OsmoDoc.csproj, replacing the oldDocumentServicereference. This aligns with the new library structure.OsmoDoc.API/Helpers/CommonMethodsHelper.cs (1)
1-1: Namespace update
Renamed toOsmoDoc.API.Helpersto match the overall project namespace changes.OsmoDoc.API/OsmoDoc.API.sln (1)
6-6: Solution project entry updated
The solution file now correctly referencesOsmoDoc.API.csprojunder theOsmoDoc.APIname, replacing the oldDocumentService.API.OsmoDoc.API/Models/PdfGenerationRequestDTO.cs (2)
1-1: Using directive updated
Theusing OsmoDoc.Pdf.Models;statement has been updated to reflect the new assembly.
4-4: Namespace update
The DTO namespace is nowOsmoDoc.API.Models, consistent with the project's renaming.docs/site/10.0.2/api/OsmoDocWord.html (3)
8-9: Approve head metadata namespace update
The<title>andmeta name="title"entries have been correctly updated to reflect theOsmoDoc.Wordnamespace.Also applies to: 11-12
70-70: Approve article identifiers and header update
Thedata-uidon the<article>and the<h1>have been consistently renamed toOsmoDoc.Word.Also applies to: 72-73
80-80: Approve class link update
The hyperlink toWordDocumentGeneratornow correctly points toOsmoDoc.Word.WordDocumentGenerator.html.docs/site/10.0.2/api/OsmoDoc.Word.Models.html (4)
8-9: Approve head metadata namespace update
The<title>andmeta name="title"entries correctly referenceOsmoDoc.Word.Models.Also applies to: 11-12
70-70: Approve article identifiers and header update
The<article>data-uidand<h1>have been updated toOsmoDoc.Word.Models.Also applies to: 72-73
80-84: Approve class links update
The class reference links (ContentData,DocumentData,TableData) have been updated to the newOsmoDoc.Word.Models.*filenames.
89-91: Approve enum links update
The enum reference links (ContentType,ParentBody) correctly point toOsmoDoc.Word.Models.*pages.OsmoDoc/Pdf/Models/ContentMetaData.cs (1)
1-1: Approve namespace declaration
The file-scoped namespaceOsmoDoc.Pdf.Modelsis correctly set for this new model class.OsmoDoc.API/Helpers/AutoMappingProfile.cs (2)
1-3: Approve namespace and using updates
Theusingdirectives and the file-scoped namespace have been correctly updated toOsmoDoc.Word.ModelsandOsmoDoc.API.Helpers.Also applies to: 5-5
11-11: Approve AutoMapper configuration
TheCreateMap<WordContentDataRequestDTO, ContentData>()mapping aligns with the renamed model and DTO namespaces.docs/site/10.0.2/api/OsmoDoc.Word.Models.ParentBody.html (4)
70-70: Approve articledata-uidupdate
The<article>now targetsOsmoDoc.Word.Models.ParentBody.
75-75: Approve headerdata-uidand text update
The<h1>has been updated to reflectEnum ParentBodyunderOsmoDoc.Word.Models.ParentBody.
79-80: Approve namespace and assembly metadata
The namespace and assembly links/text have been correctly updated toOsmoDoc.Word.ModelsandOsmoDoc.dll.
96-96: Approve table ID updates
The<td>elements for enum fieldsNoneandTablehave been updated with the correct IDs underOsmoDoc.Word.Models.ParentBody.Also applies to: 100-100
docs/site/10.0.2/api/toc.html (1)
17-43: Consistent namespace update in TOC linksAll API navigation links have been correctly renamed from
DocumentServicetoOsmoDocand the hrefs, titles, and link texts now consistently reflect the newOsmoDocnamespaces. No discrepancies found.docs/site/10.0.2/api/OsmoDoc.Word.Models.TableData.html (1)
70-115: Namespace and UID alignment is correctThe
data-uid, element IDs, and namespace/assembly references have been updated consistently toOsmoDoc.Word.Models.TableData. All links and identifiers align with the new project naming.docs/site/manifest.json (1)
8-82: Manifest file entries updated for renamed namespacesAll
source_relative_pathandoutput.relative_pathentries correctly reference theOsmoDocYAML and HTML files, replacing the oldDocumentServiceprefix. This aligns with the broader project rename.docs/site/10.0.2/api/OsmoDoc.Word.Models.ContentType.html (1)
70-100: Enum documentation renamed correctlyThe
data-uid, element IDs, and namespace/assembly declarations now consistently useOsmoDoc.Word.Models.ContentType. Enum field IDs (Image,Text) match the new UID scheme.docs/site/10.0.2/api/OsmoDoc.Word.WordDocumentGenerator.html (1)
70-116: Generated documentation ‑ no actionable code changesThis hunk only contains namespace/assembly renaming inside a DocFX-generated HTML file. No runtime code or author-maintained content is affected, so nothing to review here.
OsmoDoc.sln (1)
6-9: Solution entries look consistentBoth projects were renamed correctly and GUIDs remained untouched. No issues detected.
docs/site/10.0.2/api/OsmoDoc.Word.Models.DocumentData.html (1)
70-157: Generated documentation ‑ no actionable code changesSame as above – purely DocFX output reflecting the new namespace; nothing to review.
OsmoDoc/Word/Models/Enums.cs (1)
1-35: Enums look goodThe naming, XML docs and value choices follow .NET conventions. No issues spotted.
OsmoDoc.API/Models/WordGenerationRequestDTO.cs (1)
22-24: 👍 Default-initialised collections prevent NREs
HavingPlaceholdersandTablesDataeagerly initialised keeps downstream code simpler and safer.docs/site/xrefmap.yml (1)
4-60: Documentation sync looks correct.
All UIDs now point toOsmoDoc.*; no danglingDocumentServicereferences observed in this excerpt.OsmoDoc.API/Program.cs (1)
26-30: Path to .env may break in container
Path.Combine(root, "..", ".env")points outside the working directory once inside the container. Load the file relative toAppContext.BaseDirectoryor rely on DockerENVinstead.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
OsmoDoc/Word/WordDocumentGenerator.cs (4)
135-142: Follow async naming conventions and use genuine async operations.The method name should end with "Async" and should use genuine async file operations instead of wrapping synchronous operations in
Task.Run.- private async static Task<XWPFDocument> GetXWPFDocument(string docFilePath) + private static async Task<XWPFDocument> GetXWPFDocumentAsync(string docFilePath) { - return await Task.Run(() => - { - FileStream readStream = File.OpenRead(docFilePath); - return new XWPFDocument(readStream); - }); + using (FileStream readStream = new FileStream(docFilePath, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, useAsync: true)) + { + byte[] fileBytes = new byte[readStream.Length]; + await readStream.ReadAsync(fileBytes, 0, fileBytes.Length); + using (MemoryStream memStream = new MemoryStream(fileBytes)) + { + return new XWPFDocument(memStream); + } + } }Also update the method call on line 73:
- XWPFDocument document = await GetXWPFDocument(templateFilePath); + XWPFDocument document = await GetXWPFDocumentAsync(templateFilePath);
149-164: Apply async naming conventions to WriteDocument method.Similar to the previous method, this should be renamed to
WriteDocumentAsyncand use genuine async operations.- private async static Task WriteDocument(XWPFDocument document, string filePath) + private static async Task WriteDocumentAsync(XWPFDocument document, string filePath) { string? directory = System.IO.Path.GetDirectoryName(filePath); if (!string.IsNullOrWhiteSpace(directory)) { Directory.CreateDirectory(directory); } - await Task.Run(() => - { - using (FileStream writeStream = File.Create(filePath)) - { - document.Write(writeStream); - } - }); + using (FileStream writeStream = new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None, 4096, useAsync: true)) + { + document.Write(writeStream); + } }Also update the method call on line 112:
- await WriteDocument(document, outputFilePath); + await WriteDocumentAsync(document, outputFilePath);
307-313: Add comprehensive null checks to prevent runtime exceptions.The static analysis correctly identifies potential null reference issues that could cause runtime exceptions.
- if (docProperty != null && imagePlaceholders.ContainsKey(docProperty.Name)) + if (docProperty != null && !string.IsNullOrEmpty(docProperty.Name) && imagePlaceholders.ContainsKey(docProperty.Name)) { List<Blip> drawingBlips = drawing.Descendants<Blip>().ToList(); foreach (Blip blip in drawingBlips) { + if (wordDocument.MainDocumentPart == null || string.IsNullOrEmpty(blip.Embed)) + { + continue; + } OpenXmlPart imagePart = wordDocument.MainDocumentPart.GetPartById(blip.Embed);
316-318: Enhance image handling to support both HTTP URLs and local file paths.The current implementation only handles HTTP URLs. Consider supporting local file paths as well for more flexibility.
string imagePath = imagePlaceholders[docProperty.Name]; - // Asynchronously download image data using HttpClient - byte[] imageData = await _httpClient.GetByteArrayAsync(imagePath); + byte[] imageData; + if (Uri.TryCreate(imagePath, UriKind.Absolute, out Uri? imageUri) && + (imageUri.Scheme == Uri.UriSchemeHttp || imageUri.Scheme == Uri.UriSchemeHttps)) + { + // Download from HTTP/HTTPS URL + imageData = await _httpClient.GetByteArrayAsync(imagePath); + } + else + { + // Read from local file + imageData = await File.ReadAllBytesAsync(imagePath); + }
🧹 Nitpick comments (3)
.env.example (2)
1-1: Clarify JWT_KEY placeholder and add guidance
The placeholder highlights the need for a strong, 32+ character secret. Consider using a clearer placeholder (e.g.,YOUR_SECRET_KEY_MIN_32_CHARS) and adding a comment reminding users not to commit real secrets and to load this via a secure secrets manager in production.
3-4: Consolidate and standardize Redis connection vars
Maintaining bothREDIS_URL(with port embedded) andREDIS_PORTcan be redundant and may confuse users. Consider either:
- Splitting into
REDIS_HOST=redis+REDIS_PORT=6379- Or using a single
REDIS_CONNECTION_STRING=redis:6379Also, reorder/group related keys (e.g., host before port) to satisfy dotenv-linter ordering rules.
OsmoDoc/Word/WordDocumentGenerator.cs (1)
23-23: Consider HttpClient lifecycle management.The static HttpClient field may cause issues in certain scenarios. Consider using dependency injection or IHttpClientFactory for better lifecycle management, especially in web applications.
Consider refactoring to accept an HttpClient parameter:
- private static readonly HttpClient _httpClient = new HttpClient(); - public async static Task GenerateDocumentByTemplate(string templateFilePath, DocumentData documentData, string outputFilePath) + public async static Task GenerateDocumentByTemplate(string templateFilePath, DocumentData documentData, string outputFilePath, HttpClient? httpClient = null)Then use the provided client or create a using block when needed for HTTP operations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.env.example(1 hunks)OsmoDoc.API/Controllers/LoginController.cs(1 hunks)OsmoDoc.API/Controllers/PdfController.cs(1 hunks)OsmoDoc.API/Controllers/WordController.cs(1 hunks)OsmoDoc.API/Models/LoginRequestDTO.cs(1 hunks)OsmoDoc.API/Models/RevokeTokenRequestDTO.cs(1 hunks)OsmoDoc.API/OsmoDoc.API.csproj(1 hunks)OsmoDoc.API/Program.cs(1 hunks)OsmoDoc/OsmoDoc.csproj(1 hunks)OsmoDoc/Pdf/PdfDocumentGenerator.cs(1 hunks)OsmoDoc/Services/Interfaces/IRedisTokenStoreService.cs(1 hunks)OsmoDoc/Services/RedisTokenStoreService.cs(1 hunks)OsmoDoc/Word/WordDocumentGenerator.cs(1 hunks)docker-compose.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- OsmoDoc/Services/Interfaces/IRedisTokenStoreService.cs
🚧 Files skipped from review as they are similar to previous changes (10)
- OsmoDoc.API/OsmoDoc.API.csproj
- OsmoDoc.API/Controllers/WordController.cs
- OsmoDoc.API/Models/LoginRequestDTO.cs
- OsmoDoc/OsmoDoc.csproj
- OsmoDoc.API/Controllers/PdfController.cs
- OsmoDoc.API/Models/RevokeTokenRequestDTO.cs
- docker-compose.yaml
- OsmoDoc.API/Program.cs
- OsmoDoc/Services/RedisTokenStoreService.cs
- OsmoDoc.API/Controllers/LoginController.cs
🧰 Additional context used
🪛 GitHub Check: build
OsmoDoc/Word/WordDocumentGenerator.cs
[failure] 151-151:
'Path' is an ambiguous reference between 'DocumentFormat.OpenXml.Drawing.Path' and 'System.IO.Path'
[warning] 313-313:
Possible null reference argument for parameter 'id' in 'OpenXmlPart OpenXmlPartContainer.GetPartById(string id)'.
[warning] 313-313:
Dereference of a possibly null reference.
[warning] 307-307:
Possible null reference argument for parameter 'key' in 'bool Dictionary<string, string>.ContainsKey(string key)'.
[failure] 151-151:
'Path' is an ambiguous reference between 'DocumentFormat.OpenXml.Drawing.Path' and 'System.IO.Path'
🪛 GitHub Actions: Build and Test
OsmoDoc/Word/WordDocumentGenerator.cs
[error] 151-151: CS0104: 'Path' is an ambiguous reference between 'DocumentFormat.OpenXml.Drawing.Path' and 'System.IO.Path'.
🪛 GitHub Actions: PR #42
OsmoDoc/Word/WordDocumentGenerator.cs
[error] 151-151: CS0104: 'Path' is an ambiguous reference between 'DocumentFormat.OpenXml.Drawing.Path' and 'System.IO.Path'.
[warning] 307-307: CS8604: Possible null reference argument for parameter 'key' in 'bool Dictionary<string, string>.ContainsKey(string key)'.
[warning] 313-313: CS8602: Dereference of a possibly null reference.
[warning] 313-313: CS8604: Possible null reference argument for parameter 'id' in 'OpenXmlPart OpenXmlPartContainer.GetPartById(string id)'.
[warning] 315-315: CS8604: Possible null reference argument for parameter 'key' in 'string Dictionary<string, string>.this[string key]'.
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 4-4: [UnorderedKey] The REDIS_PORT key should go before the REDIS_URL key
🔇 Additional comments (2)
OsmoDoc/Pdf/PdfDocumentGenerator.cs (2)
196-199: Previous null handling issue has been resolved.The code now properly handles the potential null
ejsDataJsonby using the null-coalescing operator to provide a default empty JSON object"{}", which addresses the previous review concern about potentialArgumentNullException.
23-82: Well-structured PDF generation method with comprehensive validation.The method includes proper input validation, platform-specific handling, and clear separation of concerns between EJS conversion and HTML-to-PDF processing. The error handling appropriately propagates exceptions for caller handling.
…ding unique id in the filename
osm-Jatin
force-pushed
the
feat/redis
branch
3 times, most recently
from
8672908 to
1057e4d
Compare
…ve generated token to redis
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
OsmoDoc/Word/WordDocumentGenerator.cs (1)
303-309: Add null checks to prevent potential NullReferenceException.The static analysis correctly identifies potential null reference issues that need to be addressed.
- if (docProperty != null && imagePlaceholders.ContainsKey(docProperty.Name)) + if (docProperty != null && !string.IsNullOrEmpty(docProperty.Name) && imagePlaceholders.ContainsKey(docProperty.Name)) { List<Blip> drawingBlips = drawing.Descendants<Blip>().ToList(); foreach (Blip blip in drawingBlips) { + if (wordDocument.MainDocumentPart == null || string.IsNullOrEmpty(blip.Embed)) + { + continue; + } OpenXmlPart imagePart = wordDocument.MainDocumentPart.GetPartById(blip.Embed);
🧹 Nitpick comments (6)
.env.example (2)
1-1: Enhance JWT key security and naming consistency.The placeholder note is helpful, but consider aligning the env var name with your codebase (e.g.
JWT_SECRETorOSMODOC_JWT_SECRET) and emphasize a true 32-byte secret. Also ensure.envis listed in.gitignoreso real keys aren’t committed.
3-4: Optimize Redis variables and satisfy dotenv-linter.To address the UnorderedKey warning and reduce duplication, reorder
REDIS_PORTbeforeREDIS_URLand consider splitting host/port, then deriving the URL. For example:-REDIS_URL=redis:6379 -REDIS_PORT=6379 +REDIS_PORT=6379 +REDIS_URL=redis:${REDIS_PORT}Or even:
+REDIS_HOST=redis +REDIS_PORT=6379 REDIS_URL=${REDIS_HOST}:${REDIS_PORT}OsmoDoc/Word/WordDocumentGenerator.cs (4)
30-30: Follow async naming conventions.Methods that return
Taskshould have the "Async" suffix according to .NET conventions.- public async static Task GenerateDocumentByTemplate(string templateFilePath, DocumentData documentData, string outputFilePath) + public async static Task GenerateDocumentByTemplateAsync(string templateFilePath, DocumentData documentData, string outputFilePath)
296-297: Address the FIXME comment for performance optimization.The nested loop structure could potentially be optimized, especially for documents with many images.
Would you like me to suggest an optimized approach using LINQ or dictionary lookups to improve the performance of this image replacement operation?
314-315: Consider extracting HttpClient to avoid repeated instantiation.Creating a new
HttpClientinstance for each image can be inefficient and may lead to socket exhaustion under heavy load.Consider one of these approaches:
- Extract
HttpClientas a static readonly field- Use
IHttpClientFactoryif this is part of a DI container- Pass
HttpClientas a parameter to the method+ private static readonly HttpClient _httpClient = new HttpClient(); + private async static Task ReplaceImagePlaceholders(string inputFilePath, string outputFilePath, Dictionary<string, string> imagePlaceholders) { // ... existing code ... - using HttpClient httpClient = new HttpClient(); - byte[] imageData = await httpClient.GetByteArrayAsync(imagePath); + byte[] imageData = await _httpClient.GetByteArrayAsync(imagePath);
83-83: Consider using a compiled regex for better performance.The regex pattern
@"{[a-zA-Z]+}"is used multiple times and could benefit from compilation for better performance in document processing scenarios.+ private static readonly Regex PlaceholderRegex = new Regex(@"{[a-zA-Z]+}", RegexOptions.Compiled); + // Then replace the inline regex usage: - if (paragraph.ParagraphText == string.Empty || !new Regex(@"{[a-zA-Z]+}").IsMatch(paragraph.ParagraphText)) + if (paragraph.ParagraphText == string.Empty || !PlaceholderRegex.IsMatch(paragraph.ParagraphText)) - List<string> placeholdersTobeReplaced = Regex.Matches(paragraph.ParagraphText, @"{[a-zA-Z]+}") + List<string> placeholdersTobeReplaced = PlaceholderRegex.Matches(paragraph.ParagraphText)Also applies to: 204-204
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.env.example(1 hunks)OsmoDoc.API/Controllers/LoginController.cs(1 hunks)OsmoDoc.API/Controllers/PdfController.cs(1 hunks)OsmoDoc.API/Models/LoginRequestDTO.cs(1 hunks)OsmoDoc.API/Models/RevokeTokenRequestDTO.cs(1 hunks)OsmoDoc.API/OsmoDoc.API.csproj(1 hunks)OsmoDoc.API/Program.cs(1 hunks)OsmoDoc/OsmoDoc.csproj(1 hunks)OsmoDoc/Pdf/PdfDocumentGenerator.cs(1 hunks)OsmoDoc/Services/Interfaces/IRedisTokenStoreService.cs(1 hunks)OsmoDoc/Services/RedisTokenStoreService.cs(1 hunks)OsmoDoc/Word/WordDocumentGenerator.cs(1 hunks)docker-compose.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- OsmoDoc.API/OsmoDoc.API.csproj
- OsmoDoc/OsmoDoc.csproj
- OsmoDoc.API/Models/LoginRequestDTO.cs
- OsmoDoc.API/Controllers/PdfController.cs
- OsmoDoc.API/Models/RevokeTokenRequestDTO.cs
- OsmoDoc/Services/Interfaces/IRedisTokenStoreService.cs
- OsmoDoc.API/Program.cs
- OsmoDoc/Services/RedisTokenStoreService.cs
- OsmoDoc.API/Controllers/LoginController.cs
- OsmoDoc/Pdf/PdfDocumentGenerator.cs
- docker-compose.yaml
🧰 Additional context used
🪛 GitHub Check: build
OsmoDoc/Word/WordDocumentGenerator.cs
[warning] 309-309:
Possible null reference argument for parameter 'id' in 'OpenXmlPart OpenXmlPartContainer.GetPartById(string id)'.
[warning] 309-309:
Dereference of a possibly null reference.
[warning] 303-303:
Possible null reference argument for parameter 'key' in 'bool Dictionary<string, string>.ContainsKey(string key)'.
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 4-4: [UnorderedKey] The REDIS_PORT key should go before the REDIS_URL key
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (2)
OsmoDoc/Word/WordDocumentGenerator.cs (2)
1-16: Good use of alias to resolve namespace conflicts.The
IOPath = System.IO.Pathalias effectively resolves the ambiguous reference betweenDocumentFormat.OpenXml.Drawing.PathandSystem.IO.Path, addressing the compilation error from previous reviews.
134-138: Async implementation looks good.The method correctly uses
File.ReadAllBytesAsyncfor async file reading, which addresses the previous suggestion about using proper async file operations instead ofTask.Run.
…t.RequestAborted for automatic request cancellation
Task Link
REST-1566
Description
Summary by CodeRabbit