Conversation
WalkthroughThis update centralizes CORS (Cross-Origin Resource Sharing) configuration by introducing a global CORS setup via a new Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Filter (JwtUserIdValidationFilter)
participant Spring (CorsConfig)
participant Controller
Browser->>Filter: Sends HTTP request with Origin header
alt OPTIONS preflight
Filter->>Filter: Check if Origin is allowed
alt Origin allowed
Filter->>Browser: Respond 200 OK with CORS headers
else Origin not allowed
Filter->>Browser: Respond 200 OK without CORS headers
end
else Non-OPTIONS request
Filter->>Filter: Check if Origin is allowed
alt Origin allowed
Filter->>Spring: Pass request with CORS headers
else Origin not allowed
Filter->>Spring: Pass request without CORS headers
end
Spring->>Controller: Dispatch to endpoint
Controller->>Browser: Return response
end
Suggested reviewers
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: 3
🔭 Outside diff range comments (2)
src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackSeverity.java (1)
46-52: Missing injection for FeedbackTypeService.
ThefeedbackTypeServicefield is never wired, which will cause an NPE ingetFeedbackType(). Add an@Autowiredsetter or field injection:+ @Autowired + public void setFeedbackTypeService(FeedbackTypeService feedbackTypeService) { + this.feedbackTypeService = feedbackTypeService; + }src/main/java/com/iemr/helpline1097/controller/beneficiarycall/Service1097HistoryController.java (1)
148-151: Potential negativepageNowhen client sends0
pageNo = requestObject.has("pageNo") ? (requestObject.getInt("pageNo") - 1) : 0;
If the caller legitimately passes0, this yields-1, which could break downstream paging logic. Clamp to0after the subtraction:-int pageNo = requestObject.has("pageNo") ? (requestObject.getInt("pageNo") - 1) : 0; +int pageNo = requestObject.has("pageNo") ? Math.max(requestObject.getInt("pageNo") - 1, 0) : 0;Also applies to: 149-151
🧹 Nitpick comments (8)
src/main/java/com/iemr/helpline1097/controller/co/services/CategoryController.java (1)
47-49: Fix typo in summary and method name.
The operation summary and method name currently read"categries". Correct to"categories"for clarity and consistency:- @Operation(summary = "Get all categries") - public String getAllCategries() { + @Operation(summary = "Get all categories") + public String getAllCategories() {src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackSeverity.java (2)
55-58: Fix operation summary for severity endpoint.
ThegetSeverity()method summary is"Get feedback type". It should be"Get feedback severity":- @Operation(summary = "Get feedback type") + @Operation(summary = "Get feedback severity")
49-52: Normalize setter method name.
RenameSetFeedbackSeverityServicetosetFeedbackSeverityService(lowercase 's') to comply with Java naming conventions:- public void SetFeedbackSeverityService(FeedbackSeverityService feedbackSeverityService) { + public void setFeedbackSeverityService(FeedbackSeverityService feedbackSeverityService) {src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackController.java (1)
115-116: Guard against null before logging
Adding a null check prevents loggingnullresults. Consider also handling thenullcase in the response body if that represents an error or absence of data.src/main/java/com/iemr/helpline1097/controller/co/beneficiary/BeneficiaryController.java (1)
97-102: Duplicate JSON-deserialise block – consider local helper to reduce copy-paste
BothsaveBenCalServiceCatSubcatMappingandsaveBenCalServiceCOCatSubcatMappingnow share the exact three-line ObjectMapper → array → asList pattern. Factoring this into a small private utility will keep future fixes in one place and shorten methods.src/main/java/com/iemr/helpline1097/controller/beneficiarycall/Service1097HistoryController.java (1)
111-113: Minor nit –objectMapper.readValuecan be single-line; current split is purely stylistic
No functional change, just pointing out that the wrapped line could be reverted for readability.src/main/java/com/iemr/helpline1097/utils/JwtUserIdValidationFilter.java (2)
8-8: Remove unusedorg.springframework.stereotype.ComponentimportThe class is no longer annotated with
@Component.
Keeping this import will raise an “unused import” compile warning (and most build setups treat warnings as errors).-import org.springframework.stereotype.Component;
25-31: Pre-compute allowed origin patterns once
isOriginAllowed()re-splits the comma-separated list on every request, adding avoidable GC pressure to a hot path.
Compute the regex patterns in the constructor and store them in an immutableList<Pattern>.-private final String allowedOrigins; +private final List<Pattern> allowedOriginPatterns; ... -public JwtUserIdValidationFilter(JwtAuthenticationUtil jwtAuthenticationUtil, String allowedOrigins) { - this.jwtAuthenticationUtil = jwtAuthenticationUtil; - this.allowedOrigins = allowedOrigins; +public JwtUserIdValidationFilter(JwtAuthenticationUtil jwtAuthenticationUtil, String allowedOrigins) { + this.jwtAuthenticationUtil = jwtAuthenticationUtil; + this.allowedOriginPatterns = Arrays.stream(allowedOrigins.split(",")) + .map(String::trim) + .map(this::toRegex) + .map(Pattern::compile) + .toList(); }This removes string-processing per request and clarifies intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/helpline1097/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/beneficiarycall/Service1097HistoryController.java(3 hunks)src/main/java/com/iemr/helpline1097/controller/co/beneficiary/BeneficiaryController.java(3 hunks)src/main/java/com/iemr/helpline1097/controller/co/callhandling/CalltypeController.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackController.java(4 hunks)src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackRequestController.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackSeverity.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/co/services/CategoryController.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/co/services/CommonController.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/co/services/DesignationController.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/co/services/InstitutionController.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/everwell/EverwellGuidelinesController.java(1 hunks)src/main/java/com/iemr/helpline1097/controller/version/VersionController.java(1 hunks)src/main/java/com/iemr/helpline1097/utils/FilterConfig.java(1 hunks)src/main/java/com/iemr/helpline1097/utils/JwtUserIdValidationFilter.java(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/iemr/helpline1097/config/CorsConfig.java (1)
src/main/java/com/iemr/helpline1097/utils/FilterConfig.java (1)
Configuration(9-28)
src/main/java/com/iemr/helpline1097/utils/FilterConfig.java (1)
src/main/java/com/iemr/helpline1097/config/CorsConfig.java (1)
Configuration(9-28)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (20)
src/main/environment/common_ci.properties (1)
40-42: Ensure consistency across environment configuration files.
The newcors.allowed-originsproperty is added here. Verify that the same property exists incommon_example.properties(and any other env-specific files) with a sensible default value and that its expected format (e.g., comma-separated origins) is documented.src/main/java/com/iemr/helpline1097/controller/co/services/DesignationController.java (1)
26-28: Confirm removal of method-level CORS annotations.
All@CrossOriginimports and annotations have been removed. Ensure that the globalCorsConfignow applies CORS settings (allowed origins, headers includingAuthorization, credentials, and preflight handling) to this endpoint.src/main/java/com/iemr/helpline1097/controller/co/services/InstitutionController.java (1)
24-28: Verify global CORS mapping covers this endpoint.
With the per-controller@CrossOriginremoved, confirm thatCorsConfigregisters the/api/helpline1097/co/get/institutionspath (GET) for the correct origins, headers, methods, and credentials, and handles OPTIONS preflight.src/main/java/com/iemr/helpline1097/controller/co/services/CategoryController.java (1)
30-34: Confirm removal of CrossOrigin import and annotation.
Ensure the global CORS configuration now handles POST requests to/api/helpline1097/v1/get/category, including preflight OPTIONS, allowed headers (e.g.,Authorization), and credentials support.src/main/environment/common_example.properties (1)
42-42: Verify wildcard origin pattern usage
Spring’sallowedOriginsdoesn't support port wildcards (e.g.,http://localhost:*). If you need to allow dynamic ports, switch to usingallowedOriginPatternsin yourCorsConfigor enumerate explicit origins.src/main/java/com/iemr/helpline1097/controller/co/callhandling/CalltypeController.java (1)
27-27: Removed redundant@CrossOriginimport
Per-controller CORS annotations are now centralized inCorsConfig, so this import can be dropped.
Please verify that the global CORS configuration covers/call/**endpoints as intended.src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackRequestController.java (1)
27-27: Removed unused@CrossOriginimport
CORS is handled globally; this import is no longer needed.
Confirm that/iEMR/**routes are included in yourCorsConfigmappings.src/main/java/com/iemr/helpline1097/controller/co/services/CommonController.java (1)
29-29: Dropped obsolete@CrossOriginimport
Centralized CORS configuration replaces per-controller annotations, so this import can be removed.
Ensure/api/helpline1097/co/get/**is covered by the global CORS policy.src/main/java/com/iemr/helpline1097/controller/co/feedback/FeedbackController.java (4)
29-29: Removed stale@CrossOriginimport
Method-level CORS annotations have been removed in favor of the centralized configuration.
Verify that/co/**endpoints are correctly exposed under the new CORS setup.
76-76: Eliminated method-level CORS annotation
The@CrossOriginon the “Get feedback list” endpoint was removed to unify CORS logic.
93-93: Removed per-method CORS annotation
The@CrossOriginon the “Get feedback by post” endpoint is no longer needed.
108-108: Dropped CORS annotation on save endpoint
Central CORS configuration now covers the “Save beneficiary feedback” route.src/main/java/com/iemr/helpline1097/controller/version/VersionController.java (1)
29-33:@CrossOriginimport removal looks correct
The cleanup is in line with the new global CORS strategy – nothing else to flag here.src/main/java/com/iemr/helpline1097/controller/everwell/EverwellGuidelinesController.java (1)
24-30: Import pruning is fine; verify controller still covered by global CORS
Now that per-method CORS annotations are gone, ensurecors.allowed-originsis populated in every active profile; otherwise pre-flight requests will fail before reaching this controller.src/main/java/com/iemr/helpline1097/controller/co/beneficiary/BeneficiaryController.java (1)
29-33: CrossOrigin import removal approved
No issues detected.src/main/java/com/iemr/helpline1097/controller/beneficiarycall/Service1097HistoryController.java (1)
31-35: Import removal fine
No concerns here.src/main/java/com/iemr/helpline1097/utils/FilterConfig.java (2)
20-24: Double-CORS handling – verify header duplication
The filter now sets CORS headers whileCorsConfigregisters aWebMvcConfigurerdoing the same. DuplicateAccess-Control-Allow-Originheaders may get written, depending on servlet container behaviour. Confirm only one component commits the header (filter can skip if Spring already handled it).
24-25: Good call elevating filter order
Placing JWT/CORS logic atHIGHEST_PRECEDENCEensures pre-auth OPTIONS requests succeed before Spring Security kicks in.src/main/java/com/iemr/helpline1097/config/CorsConfig.java (1)
22-24: ```shell
#!/bin/bash1. Inspect allowed headers in CorsConfig
echo "=== CorsConfig.allowedHeaders ==="
rg -n ".allowedHeaders" -C2 src/main/java/com/iemr/helpline1097/config/CorsConfig.java2. Locate the JwtUserIdValidationFilter implementation(s)
echo "=== JwtUserIdValidationFilter file(s) ==="
rg -l "class JwtUserIdValidationFilter" -g "src/main/java/com/iemr/helpline1097/**/*.java"3. For each filter file, check which headers it advertises
for f in $(rg -l "class JwtUserIdValidationFilter" -g "src/main/java/com/iemr/helpline1097/**/*.java"); do
echo
echo "=== Inspecting $f ==="
rg -n "Access-Control-Allow-Headers" -C2 "$f"
rg -n "Jwttoken" -C2 "$f"
rg -n "Accept" -C2 "$f"
done</details> <details> <summary>src/main/java/com/iemr/helpline1097/utils/JwtUserIdValidationFilter.java (1)</summary> `44-52`: **Header list diverges from `CorsConfig`** The response advertises `Accept` and `Jwttoken`, headers not allowed by `CorsConfig`. Please keep the two components consistent (single constant or shared config) to avoid failed pre-flights. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
src/main/java/com/iemr/helpline1097/utils/JwtUserIdValidationFilter.java
Show resolved
Hide resolved
vishwab1
changed the title
2 participants
📋 Description
JIRA ID: AMM-1246
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Refactor
Chores