feat: add login endpoint #41
Conversation
… all related namespaces
|
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 21 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (4)
WalkthroughThis update rebrands the project from "DocumentService" to "OsmoDoc" across all code, configuration, documentation, and infrastructure files. It removes the old DocumentService codebase, introduces OsmoDoc equivalents with similar functionality, and updates namespaces, references, and documentation accordingly. New login functionality is added, and all public APIs, DTOs, and document generators are migrated or recreated under the new OsmoDoc structure. Changes
Sequence Diagram(s)Login FlowsequenceDiagram
participant Client
participant LoginController
participant AuthenticationHelper
Client->>LoginController: POST /api/login (LoginRequestDTO)
LoginController->>AuthenticationHelper: JwtTokenGenerator(email)
AuthenticationHelper-->>LoginController: JWT token
LoginController-->>Client: 200 OK (BaseResponse with token)
LoginController-->>Client: 500 Error (on exception)
PDF Generation (HTML/EJS)sequenceDiagram
participant Client
participant PdfController
participant PdfDocumentGenerator
participant OsmoDocPdfConfig
Client->>PdfController: POST /api/pdf (PDF request)
PdfController->>OsmoDocPdfConfig: Set WkhtmltopdfPath
PdfController->>PdfDocumentGenerator: GeneratePdf(templatePath, placeholders, ...)
PdfDocumentGenerator-->>PdfController: PDF file generated
PdfController-->>Client: 200 OK (BaseResponse)
PdfController-->>Client: 500 Error (on exception)
Word Document GenerationsequenceDiagram
participant Client
participant WordController
participant WordDocumentGenerator
Client->>WordController: POST /api/word (Word request)
WordController->>WordDocumentGenerator: GenerateDocumentByTemplate(templatePath, data, outputPath)
WordDocumentGenerator-->>WordController: Word file generated
WordController-->>Client: 200 OK (BaseResponse)
WordController-->>Client: 500 Error (on exception)
Poem
✨ 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: 22
🧹 Nitpick comments (25)
OsmoDoc.API/Models/LoginRequestDTO.cs (3)
5-5: Rename the type to follow .NET acronym-casing guidelinesPer Microsoft naming conventions, acronyms with three or more letters should be Pascal-cased (e.g.,
Dto,Html).
LoginRequestDTO➜LoginRequestDtoimproves consistency and avoids StyleCop warnings.-public class LoginRequestDTO +public class LoginRequestDto
5-10: Consider using arecordfor concise, immutable DTOsDTOs are typically immutable value carriers. A C# 9+ positional record makes that intent explicit, provides value-equality out-of-the-box, and trims boilerplate:
-public class LoginRequestDto -{ - [Required(ErrorMessage = "Email is required")] - [EmailAddress(ErrorMessage = "Invalid email format")] - public string Email { get; set; } = string.Empty; -} +public sealed record LoginRequestDto( + [property: Required(ErrorMessage = "Email is required")] + [property: EmailAddress(ErrorMessage = "Invalid email format")] + string Email +);This is a drop-in change for model binding in ASP.NET Core.
If mutability is needed, disregard this suggestion.
7-9: Optional: annotate JSON name for camel-case payloadsIf the project’s
JsonSerializerOptions.PropertyNamingPolicyisn’t already set toCamelCase, add an explicit attribute to keep the external contract stable:- [Required(ErrorMessage = "Email is required")] - [EmailAddress(ErrorMessage = "Invalid email format")] - public string Email { get; set; } = string.Empty; + [Required(ErrorMessage = "Email is required")] + [EmailAddress(ErrorMessage = "Invalid email format")] + [JsonPropertyName("email")] + public string Email { get; set; } = string.Empty;Requires
using System.Text.Json.Serialization;OsmoDoc/Pdf/Models/OsmoDocPdfConfig.cs (1)
5-6: Provide a sensible default or validateWkhtmltopdfPath.
Since the property is nullable, you may encounter null-reference errors if it isn’t set at runtime. Consider either:
- Assigning a default value (e.g.
public static string WkhtmltopdfPath { get; set; } = "wkhtmltopdf";)- Failing fast in your PDF generator if this property is null or empty.
.env.example (1)
1-1: Add a trailing newline to.env.example.
dotenv-linter flagged a missing blank line at the end of the file. Adding a newline will satisfy the linter.OsmoDoc/OsmoDoc.csproj (2)
3-7: Use<TargetFramework>for a single target.
Since this project only targetsnet8.0, updating to:<TargetFramework>net8.0</TargetFramework>will simplify the project file and follow common conventions.
13-16: Replace the<PackageProjectUrl>placeholder before release.
The<PackageProjectUrl>currently reads"URL to project homepage or documentation". Please update it to the actual project homepage or docs URL so package consumers can find relevant information.OsmoDoc.API/Helpers/Base64StringHelper.cs (2)
5-13: Handle invalid Base64 input explicitly.
Convert.FromBase64Stringwill throwFormatExceptionon malformed input, leading to a 500 response. Wrap the call in a try-catch and rethrow aBadHttpRequestExceptionto return a 400 status:- byte[] data = Convert.FromBase64String(base64String); + byte[] data; + try + { + data = Convert.FromBase64String(base64String); + } + catch (FormatException) + { + throw new BadHttpRequestException("Invalid Base64 string"); + }
9-11: Simplify configuration value retrieval.
Instead of:long uploadFileSizeLimitBytes = Convert.ToInt64(configuration.GetSection("CONFIG:UPLOAD_FILE_SIZE_LIMIT_BYTES").Value);use:
long uploadFileSizeLimitBytes = configuration.GetValue<long>("CONFIG:UPLOAD_FILE_SIZE_LIMIT_BYTES");This is more concise and handles null/default cases.
OsmoDoc/Pdf/Models/DocumentData.cs (2)
1-4: Remove unused using directive
Theusing System.Text;import is not used—consider removing it to clean up dependencies.
7-10: Add XML documentation for public API
Consider adding<summary>comments for theDocumentDataclass and itsPlaceholdersproperty to improve IDE tooling and overall maintainability.OsmoDoc.API/Controllers/WordController.cs (2)
62-98: Fail-fast validation can be extracted for clarity & re-useThe image–placeholder validation block is large, duplicated across potential callers, and clutters controller logic. Consider moving it into a helper/validator method (or FluentValidation) so the controller focuses on orchestration only.
115-120: Returning large Base64 payloads via JSON is inefficientFor anything but very small documents a Base64 string blows payload size by ~33 %. Prefer returning
FileContentResult(binaryapplication/vnd.openxmlformats-officedocument.wordprocessingml.document) or at least provide a signed temporary download URL.README.md (2)
1-2: Duplicate word – “OsmoDoc OsmoDoc”Tiny nitpick: the heading currently reads “# OsmoDoc OsmoDoc is…”. Removing the second occurrence improves readability.
52-60: Use the environment variable in the Docker command example
docker composeis correct, but many users still havedocker-compose. Consider mentioning both invocations (docker compose …/docker-compose …) to avoid confusion across older Docker installations.Dockerfile (1)
34-46: Slim image & multi-stage install for wkhtmltopdf/node would reduce final sizeInstalling Node & wkhtmltopdf in the runtime image adds ~300 MB. Consider copying the statically-linked wkhtmltopdf binary from the build stage and using
node:20-alpineas a separate layer only when actually needed.CONTRIBUTING.md (1)
66-66: Clarify pull request target remote
The instruction "Send a pull request to theosmodoc:main``" may confuse users about the remote alias. Consider specifying the repository asOsmosysSoftware/osmodocand the branch `main` for clarity.OsmoDoc/Word/Models/DocumentData.cs (1)
1-4: Remove unused using directive.
Theusing System.Text;directive on line 3 is unused and can be removed to clean up the file.OsmoDoc.API/Program.cs (3)
29-29: UseGetValue<T>for configuration.
Instead of reading.Valueand converting, use:long requestBodySizeLimitBytes = builder.Configuration.GetValue<long>("CONFIG:REQUEST_BODY_SIZE_LIMIT_BYTES");for brevity and null-safety.
47-60: Enhance Swagger JWT support.
You've added a security definition and operation filter. To document required scopes and enforce usage in the UI, consider adding:options.AddSecurityRequirement(new OpenApiSecurityRequirement { { new OpenApiSecurityScheme { Reference = new OpenApiReference { Type = ReferenceType.SecurityScheme, Id = "oauth2" } }, new string[] { } } });
70-76: Ensure JWT_KEY is secured.
The environment variableJWT_KEYis required at startup. Confirm it's set securely in Production and not checked into source control. You may also consider using a secrets manager (e.g., Azure Key Vault) for enhanced security.OsmoDoc/Pdf/PdfDocumentGenerator.cs (1)
63-67: Improve exception handling with contextual information.The empty catch block that just rethrows provides no additional context about what operation failed.
-catch (Exception) -{ - throw; -} +catch (Exception ex) +{ + throw new Exception($"Failed to generate PDF from template '{templatePath}' to '{outputFilePath}'", ex); +}OsmoDoc/Word/WordDocumentGenerator.cs (3)
74-74: Consider expanding the placeholder pattern to support more characters.The regex pattern
{[a-zA-Z]+}only matches letters. Consider allowing numbers and underscores 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))
114-117: Add context to exception handling.The empty catch block provides no additional context about the operation that failed.
-catch (Exception) -{ - throw; -} +catch (Exception ex) +{ + throw new Exception($"Failed to generate Word document from template '{templateFilePath}' to '{outputFilePath}'", ex); +}
287-288: Address the FIXME comment about loop optimization.The code contains a FIXME comment indicating a known performance issue.
Would you like me to suggest an optimized approach for this image replacement loop or create an issue to track this optimization task?
📜 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 (52)
.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/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/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/Pdf/Models/ContentMetaData.cs
- DocumentService/Word/Models/DocumentData.cs
- DocumentService/DocumentService.csproj
- DocumentService/Word/Models/Enums.cs
- DocumentService/Word/Models/ContentData.cs
- DocumentService/Pdf/PdfDocumentGenerator.cs
- DocumentService/Word/WordDocumentGenerator.cs
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
🪛 GitHub Check: build
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/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/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/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)'.
🪛 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 (43)
OsmoDoc/Pdf/Models/OsmoDocPdfConfig.cs (1)
3-7: Static PDF config looks solid.
The newOsmoDocPdfConfigstatic class withWkhtmltopdfPathprovides a centralized way to configure the wkhtmltopdf binary path. No functional issues detected..env.example (1)
1-1: Approve placeholder clarity forJWT_KEY.
The explicit placeholderPLACEHOLDER_REPLACE_WITH_STRONG_KEY_MIN_32_CHARS_BEFORE_USEclearly communicates the requirement for a secure key.OsmoDoc.API/OsmoDoc.API.csproj (1)
15-15: Project reference update is correct.
The API project now references..\OsmoDoc\OsmoDoc.csproj, aligning with the rebrand. No issues detected.OsmoDoc.API/Helpers/Base64StringHelper.cs (1)
1-3: Namespace updated as expected.
The namespaceOsmoDoc.API.Helpersaligns with the project’s naming conventions. No changes needed..github/PULL_REQUEST_TEMPLATE/pull_request_template_api.md (1)
5-5: Confirm updated contributing link
The PR template now points to the correctosmodocrepository guidelines.OsmoDoc.API/OsmoDoc.API.sln (1)
6-6: Renamed solution project entry
The solution file now correctly referencesOsmoDoc.API.csprojunder theOsmoDoc.APIproject name.OsmoDoc.API/Models/PdfGenerationRequestDTO.cs (1)
1-4: Align namespace and model reference
Theusingdirective and namespace were updated toOsmoDocto match the new project structure. Everything looks consistent.docs/site/10.0.2/api/OsmoDoc.Word.Models.html (4)
8-12: Documentation header updated to reflect new namespace
The<title>and<meta name="title">tags now referenceOsmoDoc.Word.Modelsinstead of the old namespace.
70-73: Article container and H1 identifiers renamed
Thearticleelement’sdata-uidand the<h1>id/data-uidattributes have been updated toOsmoDoc.Word.Models.
80-84: Model links aligned with new namespace
All<a class="xref">links now point toOsmoDoc.Word.Models.*pages. Please verify that each target file exists underdocs/site/10.0.2/api/.
89-92: Enum links aligned with new namespace
Enum documentation links have been updated toOsmoDoc.Word.Models.*. Confirm that each referenced HTML file is present in the docs folder.docs/site/10.0.2/api/OsmoDocWord.html (3)
8-12: Documentation header updated forOsmoDoc.Wordnamespace
The<title>and<meta name="title">tags correctly reflect the newOsmoDoc.Wordnamespace.
70-73: Article container and H1 identifiers updated
Thearticleelement’sdata-uidand the<h1>id/data-uidattributes now referenceOsmoDoc.Word.
80-80: Class link updated
The link toWordDocumentGenerator.htmlmatches the new namespace. Ensure this file is generated and present indocs/site/10.0.2/api/.docs/site/10.0.2/api/OsmoDoc.Word.Models.ParentBody.html (3)
70-75: Article container and heading IDs updated
Thearticletagdata-uidand<h1>id/data-uidnow correctly referenceOsmoDoc.Word.Models.ParentBody.
79-80: Namespace and assembly references updated
The<h6>lines now showNamespace: OsmoDoc.Word.ModelsandAssembly: OsmoDoc.dll, aligning with the new project naming.
96-100: Enum field IDs updated
The<td id>attributes forParentBody.NoneandParentBody.Tablehave been renamed to include theOsmoDoc.Word.Models.ParentBodyprefix.docs/site/10.0.2/api/OsmoDoc.Word.Models.ContentType.html (3)
70-75: Article container and heading IDs updated
Thearticleelement’sdata-uidand the<h1>identifiers now referenceOsmoDoc.Word.Models.ContentType.
79-80: Namespace and assembly references updated
The documentation correctly displaysNamespace: OsmoDoc.Word.ModelsandAssembly: OsmoDoc.dll.
96-100: Enum field IDs updated
Each<td id>forImageandTextnow uses theOsmoDoc.Word.Models.ContentTypeprefix.OsmoDoc.API/Controllers/WordController.cs (2)
100-113: Mapper usage hides potential mapping failuresIf
ContentDatagains new non-nullable members the silentMap<List<ContentData>>will set them to default and document generation will break at runtime. AddMapperConfiguration.AssertConfigurationIsValid()in startup tests or convert to an explicitSelect.
28-34: Controller now requires authentication – ensure the newLoginendpoint is wired into Swagger & CORSAfter adding
[Authorize], unauthenticated integration tests and existing clients will fail unless the new/loginendpoint is documented and CORS headers allow it.docs/site/10.0.2/api/OsmoDoc.Word.WordDocumentGenerator.html (1)
70-123: API docs are out-of-sync with implementationDocumentation still shows a synchronous
void GenerateDocumentByTemplate(...), but the implementation was made async and returnsTask. Update DocFX comments or consumers will be mis-led.docs/site/10.0.2/api/OsmoDoc.Word.Models.TableData.html (1)
70-74: Docs updated to new namespace 'OsmoDoc'
All element IDs, data-uid attributes, namespace and assembly references have been correctly renamed from the oldDocumentServiceprefix toOsmoDoc.Also applies to: 107-108, 115-115, 139-139
OsmoDoc.API/Models/BaseResponse.cs (1)
1-3: Namespace and using directives updated correctly
TheMicrosoft.AspNetCore.Mvcimport and the namespaceOsmoDoc.API.Modelsalign with the new project structure and are required for theBadRequestObjectResult.docs/site/10.0.2/api/OsmoDoc.Word.Models.ContentData.html (1)
70-74: Documentation updated for namespace rename
All IDs, data-uids, cross-reference links, and assembly names have been correctly updated toOsmoDoc.Word.Models.Also applies to: 107-108, 115-115, 156-156, 179-179
OsmoDoc.API/Helpers/AutoMappingProfile.cs (1)
1-3: Namespace and using directives updated
Theusingstatements andnamespace OsmoDoc.API.Helpersreflect the new project identity. Ensure that all referenced DTOs and models (e.g.,WordContentDataRequestDTO) are in scope under these namespaces.Also applies to: 5-5
docs/site/10.0.2/api/toc.html (1)
17-17: Table of Contents links updated
All TOC entries and theirhref/titleattributes have been correctly renamed fromDocumentService.Word...toOsmoDoc.Word....Also applies to: 21-21, 27-27, 31-35, 37-43
OsmoDoc.API/DotEnv.cs (1)
1-1: Approve namespace update in DotEnv.cs
The namespace has been correctly updated fromDocumentService.APItoOsmoDoc.API, aligning with the project-wide rebranding.CONTRIBUTING.md (1)
16-18: Approve rebranding of project references and links
All references todocument-serviceand related URLs have been updated toosmodoc, maintaining link correctness and consistency across GitHub and Stack Overflow.Also applies to: 26-27, 37-38, 43-45, 49-50, 66-67
docs/site/manifest.json (1)
8-85: Approve bulk rebranding of documentation file paths
All entries underfileshave been consistently updated fromDocumentService.*toOsmoDoc.*. This aligns with the namespace and project renaming.OsmoDoc.sln (2)
6-6: Approve solution project rename
The project formerly namedDocumentServicehas been correctly renamed toOsmoDocwith the updated.csprojpath.
8-8: Approve API project rename
TheDocumentService.APIproject has been updated toOsmoDoc.API, reflecting the new folder and.csprojname.docs/site/10.0.2/api/OsmoDoc.Word.Models.DocumentData.html (1)
70-75: Approve namespace references in HTML documentation
The class, namespace, and type references have been correctly renamed fromDocumentServicetoOsmoDocin element IDs,data-uid, and headings, ensuring documentation consistency with code.Also applies to: 107-110, 115-116, 138-139
docker-compose.yaml (1)
2-2: Service rename is correct.
The service name, image, and container name have been updated toosmodoc. Ensure any CI/CD or deployment scripts referencing the olddocument-serviceare updated accordingly.Also applies to: 6-7
OsmoDoc/Word/Models/Enums.cs (1)
1-35: Enums look good.
TheContentTypeandParentBodyenums are well-defined and documented.OsmoDoc/Pdf/PdfDocumentGenerator.cs (4)
95-155: Well-implemented platform-specific PDF conversion.The method properly handles Windows and Linux platforms, manages the process lifecycle correctly, and ensures cleanup of temporary files.
225-236: Good JSON validation implementation.The method correctly validates JSON using
JToken.Parsewith appropriate exception handling.
238-252: Clean platform-specific argument generation.The method properly formats EJS command arguments for each platform with appropriate error handling for unsupported systems.
75-75: Verify placeholder pattern doesn't conflict with template engines.The double curly brace pattern
{{placeholder}}might conflict with common templating engines (e.g., Handlebars, Angular). Ensure this is intentional and documented.#!/bin/bash # Search for any documentation or tests that show the expected placeholder format fd -e md -e txt README | xargs rg -i "placeholder|template.*format" -A 3 -B 3OsmoDoc/Word/WordDocumentGenerator.cs (1)
23-23: Good use of static HttpClient instance.Using a static HttpClient instance prevents socket exhaustion and follows best practices.
docs/site/xrefmap.yml (2)
4-196: Consistent rebranding of Word namespaces.All
uid,name,href,commentId,fullName, andnameWithTypeentries have been uniformly updated fromDocumentService.WordtoOsmoDoc.Word, and no residualDocumentServicereferences remain. The ordering underreferences:appears correctly sorted by UID.
1-196: Couldn’t find anyLoginreferences in the XRef map—let’s check if any API entries at all made it into this file first:#!/bin/bash # Look for any OsmoDoc.Api entries in the cross-reference map rg -n 'OsmoDoc\.Api' docs/site/xrefmap.yml
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
OsmoDoc/Word/WordDocumentGenerator.cs (4)
175-176: Avoid unconditionally zeroingSpacingAfter(already raised).
This still risks altering author-supplied document formatting. Make it opt-in or remove altogether.
291-295: Null-safety fordocProperty.Nameis still missing.
If the name isnull,ContainsKeythrows.
298-301:blip.Embedmay be null (unchanged issue).
Add a null/empty guard beforeGetPartById.
304-306: Download URL is used without any validation (SSRF risk).
Validate the URI and optionally whitelist domains before issuing the request.
🧹 Nitpick comments (3)
.env.example (1)
1-1: Add a trailing blank line.Linting tools expect a newline at the end of
.env.example.
Apply this diff to satisfy the blank-line rule:--- a/.env.example +++ b/.env.example @@ -1 +1,2 @@ -JWT_KEY=PLACEHOLDER_REPLACE_WITH_STRONG_KEY_MIN_32_CHARS_BEFORE_USE +OsmoDoc/Word/WordDocumentGenerator.cs (2)
33-117: Thetry / catch { throw; }block is redundant noise.
catch (Exception) { throw; }does not add value—it re-throws the same exception while losing the original stack frame offset. Either remove the block or add meaningful handling / logging.
73-78: Usestring.IsNullOrEmptyand a compiled regex to shave unnecessary allocations.-if (paragraph.ParagraphText == string.Empty || !new Regex(@"{[a-zA-Z]+}").IsMatch(paragraph.ParagraphText)) +if (string.IsNullOrEmpty(paragraph.ParagraphText) || + !PlaceholderRegex.IsMatch(paragraph.ParagraphText))Add a static, compiled field:
private static readonly Regex PlaceholderRegex = new(@"{[a-zA-Z]+}", RegexOptions.Compiled);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env.example(1 hunks)OsmoDoc.API/Controllers/LoginController.cs(1 hunks)OsmoDoc.API/Controllers/WordController.cs(1 hunks)OsmoDoc.API/Models/LoginRequestDTO.cs(1 hunks)OsmoDoc/Word/WordDocumentGenerator.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- OsmoDoc.API/Controllers/WordController.cs
- OsmoDoc.API/Models/LoginRequestDTO.cs
- OsmoDoc.API/Controllers/LoginController.cs
🧰 Additional context used
🪛 GitHub Check: build
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)'.
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
🪛 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)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (1)
.env.example (1)
1-1: Placeholder updated to emphasize security requirements.The new descriptive placeholder clearly indicates the need for a strong, 32-character JWT key, aligning with the login endpoint’s security requirements.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
OsmoDoc/Word/WordDocumentGenerator.cs (5)
188-188: Avoid modifying paragraph spacing unconditionally.Setting
SpacingAfter = 0for all paragraphs with placeholders might unexpectedly alter the document's formatting.
254-265: Possible NRE when header or data cell is missing.
headerRow.GetCell(cellNumber)orrow.GetCell(cellNumber)can returnnull, which would explode atGetText()/SetText().
307-308: Fix potential null reference exceptions.The code doesn't check if
docProperty.Nameis null before using it as a dictionary key.
313-313: Add null check for blip.Embed.The static analysis correctly identifies that
blip.Embedcould be null.
318-318: Add URL validation to prevent SSRF attacks.Downloading images from arbitrary URLs without validation poses a security risk (Server-Side Request Forgery).
🧹 Nitpick comments (1)
.env.example (1)
1-1: Add trailing newline
The dotenv-linter flags a missing blank line at EOF. Please add a newline to satisfy the linter and improve compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.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/OsmoDoc.csproj(1 hunks)OsmoDoc/Pdf/PdfDocumentGenerator.cs(1 hunks)OsmoDoc/Word/WordDocumentGenerator.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- OsmoDoc/OsmoDoc.csproj
- OsmoDoc.API/Controllers/WordController.cs
- OsmoDoc.API/Controllers/LoginController.cs
- OsmoDoc.API/Controllers/PdfController.cs
- OsmoDoc.API/Models/LoginRequestDTO.cs
- OsmoDoc/Pdf/PdfDocumentGenerator.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'.
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (2)
.env.example (1)
1-1: Descriptive JWT_KEY placeholder
The updated placeholder clearly calls out the requirement for a strong, 32+ character key, which aligns with the new login endpoint’s JWT token generation.OsmoDoc/Word/WordDocumentGenerator.cs (1)
35-43: Good input validation improvements.The input validation addresses previous concerns. Consider also validating that
documentDatais not null.+if (documentData == null) +{ + throw new ArgumentNullException(nameof(documentData)); +}
…ding unique id in the filename
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
OsmoDoc/Word/WordDocumentGenerator.cs (8)
24-24: Consider dependency injection for HttpClient.Using a static HttpClient can cause DNS resolution issues and resource management problems in long-running applications.
136-143: Avoid Task.Run for I/O operations and fix resource disposal.Using
Task.Runfor I/O operations is unnecessary and consumes thread pool threads. TheFileStreamshould also be properly disposed.
189-189: Avoid modifying paragraph spacing unconditionally.Setting
SpacingAfter = 0for all paragraphs with placeholders might unexpectedly alter the document's formatting.
263-264: Possible NRE when header or data cell is missing.
headerRow.GetCell(cellNumber)orrow.GetCell(cellNumber)can returnnull, which would cause exceptions atGetText()/SetText().
310-310: Fix potential null reference exceptions.The code doesn't check if
docProperty.Nameis null before using it as a dictionary key.
316-316: Add null check for blip.Embed.The static analysis correctly identifies that
blip.Embedcould be null.
321-321: Add URL validation to prevent SSRF attacks.Downloading images from arbitrary URLs without validation poses a security risk (Server-Side Request Forgery).
46-47: Add missing null check for documentData parameter.While you've added validation for the string parameters, the
documentDataparameter is used without null checking, which could cause aNullReferenceException.+if (documentData == null) +{ + throw new ArgumentNullException(nameof(documentData)); +} + List<ContentData> contentData = documentData.Placeholders;
🧹 Nitpick comments (4)
.env.example (2)
1-1: Add a trailing newline.
Most linters and UNIX tools expect a newline at the end of files. Adding one here will satisfy theEndingBlankLinerule from dotenv-linter.
1-1: Make the JWT_KEY placeholder more explicit.
To avoid confusion and suppress generic‐API‐key alerts (e.g., from Gitleaks) in.env.example, consider using a value like:REPLACE_ME_WITH_A_SECURE_32+_CHAR_JWT_SECRETor adding a comment prefix so it’s clearly a dummy placeholder, not an actual key.
OsmoDoc/Word/WordDocumentGenerator.cs (2)
257-260: Consider optimizing cell creation.The while loop approach works but could be optimized by calculating required cells upfront and creating them in a single operation.
-while (row.GetTableCells().Count <= cellNumber) -{ - row.AddNewTableCell(); -} +// Ensure row has enough cells upfront +int requiredCells = columnCount - row.GetTableCells().Count; +for (int i = 0; i < requiredCells; i++) +{ + row.AddNewTableCell(); +}
125-128: Consider more specific exception handling.The generic catch-all exception handler with rethrow doesn't add value and masks the original exception context. Consider either removing it or handling specific exceptions appropriately.
-catch (Exception) -{ - throw; -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.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/Program.cs(1 hunks)OsmoDoc/Pdf/PdfDocumentGenerator.cs(1 hunks)OsmoDoc/Word/WordDocumentGenerator.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- OsmoDoc.API/Models/LoginRequestDTO.cs
- OsmoDoc.API/Program.cs
- OsmoDoc.API/Controllers/LoginController.cs
- OsmoDoc.API/Controllers/PdfController.cs
- OsmoDoc/Pdf/PdfDocumentGenerator.cs
🧰 Additional context used
🪛 GitHub Check: build
OsmoDoc/Word/WordDocumentGenerator.cs
[warning] 316-316:
Possible null reference argument for parameter 'id' in 'OpenXmlPart OpenXmlPartContainer.GetPartById(string id)'.
[warning] 316-316:
Dereference of a possibly null reference.
[warning] 310-310:
Possible null reference argument for parameter 'key' in 'bool Dictionary<string, string>.ContainsKey(string key)'.
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
🪛 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)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (csharp)
osm-Jatin
force-pushed
the
feat/login
branch
2 times, most recently
from
da7ff17 to
ed90f15
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
OsmoDoc/Word/WordDocumentGenerator.cs (3)
302-304: Add null check for docProperty.Name to prevent null reference exception.Despite past reviews indicating this was fixed, the current code still lacks null validation for
docProperty.Name.-if (docProperty != null && imagePlaceholders.ContainsKey(docProperty.Name)) +if (docProperty != null && !string.IsNullOrEmpty(docProperty.Name) && imagePlaceholders.ContainsKey(docProperty.Name))
309-309: Add null check for blip.Embed to prevent null reference exception.The static analysis correctly identifies that
blip.Embedcould be null, leading to a potential null reference exception.+if (string.IsNullOrEmpty(blip.Embed)) +{ + continue; +} OpenXmlPart imagePart = wordDocument.MainDocumentPart.GetPartById(blip.Embed);
255-262: Add null check for header cell access.The code assumes
headerRow.GetCell(cellNumber)will always return a valid cell, but it could be null.XWPFTableCell cell = row.GetCell(cellNumber); -string columnHeader = headerRow.GetCell(cellNumber).GetText(); +XWPFTableCell headerCell = headerRow.GetCell(cellNumber); +if (headerCell == null) continue; +string columnHeader = headerCell.GetText(); if (rowData.ContainsKey(columnHeader)) { cell.SetText(rowData[columnHeader]); }
🧹 Nitpick comments (4)
.env.example (1)
1-1: Add blank line at end of file
Include a trailing newline to satisfy.envformatting conventions and address the linter warning.Apply this diff:
JWT_KEY=PLACEHOLDER_REPLACE_WITH_STRONG_KEY_MIN_32_CHARS_BEFORE_USE +OsmoDoc/Word/WordDocumentGenerator.cs (3)
314-315: Improve HttpClient usage pattern for better resource management.Creating a new
HttpClientfor each image download is inefficient and can lead to socket exhaustion. For a NuGet package, consider using a more efficient pattern.-// Asynchronously download image data using HttpClient -using HttpClient httpClient = new HttpClient(); -byte[] imageData = await httpClient.GetByteArrayAsync(imagePath); +// Use a shared HttpClient instance for better performance +byte[] imageData = await GetImageDataAsync(imagePath);Add this helper method to the class:
private static readonly Lazy<HttpClient> _httpClient = new Lazy<HttpClient>(() => { var client = new HttpClient(); client.Timeout = TimeSpan.FromMinutes(5); return client; }); private static async Task<byte[]> GetImageDataAsync(string imagePath) { return await _httpClient.Value.GetByteArrayAsync(imagePath); }
123-126: Remove unnecessary exception handling or add meaningful error processing.The current exception handling catches all exceptions only to rethrow them, which adds no value and can make debugging harder.
- catch (Exception) - { - throw; - }
295-297: Address the FIXME comment about loop performance.The nested loops for image replacement could be optimized by building a lookup dictionary or using more efficient LINQ operations.
Would you like me to help optimize the image replacement loop performance? I can suggest more efficient alternatives to the current nested iteration pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.example(1 hunks)OsmoDoc.API/Controllers/LoginController.cs(1 hunks)OsmoDoc.API/Models/LoginRequestDTO.cs(1 hunks)OsmoDoc/Word/WordDocumentGenerator.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- OsmoDoc.API/Models/LoginRequestDTO.cs
- OsmoDoc.API/Controllers/LoginController.cs
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
🪛 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)
🪛 GitHub Check: build
OsmoDoc/Word/WordDocumentGenerator.cs
[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)'.
[warning] 146-146:
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (1)
.env.example (1)
1-1: Use a descriptive placeholder for JWT_KEY
The new placeholder clearly instructs users to replace it with a strong key of at least 32 characters, improving security guidance.
Task Link
REST-1565
Description
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation