Conversation
* feat(health,version): add health and version endpoints * fix(health): restore interrupt flag when InterruptedException occurs * fix(health): shutdown executor on destroy and sanitize infra errors in /health * fix(health): MySQL health check with timeout and sanitize errors * fix(health): harden advanced MySQL checks and throttle execution * fix(health): remove unused imports * fix(health): fux deadlock detection issue * fix(health): fix deadline timeout issue * fix(health): scope PROCESSLIST lock-wait check to application DB user * refactor(health): extract MySQL basic health query into helper method * fix(health): avoid sharing JDBC connections across threads in advanced MySQL checks * fix(health): avoid blocking DB I/O under write lock and restore interrupt flag * fix: add missing close brace
Update pom version
Fix the build error
📝 WalkthroughWalkthroughThe pull request adds health and version information endpoints to the application. A Maven plugin generates git metadata during the build process. New REST controllers expose these endpoints, while a health service performs concurrent database connectivity checks. Security configuration and JWT filters are updated to permit unauthenticated access to the new endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HealthController
participant HealthService
participant MySQL
participant Redis
participant ThreadPool
Client->>HealthController: GET /health
HealthController->>HealthService: checkHealth()
HealthService->>ThreadPool: Submit MySQL check task
HealthService->>ThreadPool: Submit Redis check task
ThreadPool->>MySQL: SELECT 1 (connectivity test)
ThreadPool->>Redis: PING (connectivity test)
MySQL-->>ThreadPool: Response / Timeout
Redis-->>ThreadPool: Response / Timeout
ThreadPool-->>HealthService: Collect results (status, responseTime)
HealthService->>HealthService: Advanced diagnostics (throttled)
HealthService->>HealthService: Aggregate component statuses<br/>(DOWN, DEGRADED, UP)
HealthService-->>HealthController: Map with overall status
HealthController->>HealthController: Determine HTTP status<br/>(200 or 503)
HealthController-->>Client: ResponseEntity with status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pom.xml`:
- Around line 21-25: The pom currently sets the Maven property
maven.test.skip=true which disables test compilation and execution; remove that
property or change it to maven.test.skip=false (or simply delete the
<maven.test.skip> element) so tests run by default, and if skipping is needed in
specific runs use CLI flags (e.g., -DskipTests or -DskipITs) rather than
committing a global default.
In `@src/main/java/com/iemr/mmu/utils/mapper/SecurityConfig.java`:
- Around line 41-43: SecurityConfig currently permits anonymous access to
"/health" and "/version", exposing detailed payloads from HealthController and
VersionController; update the authorization rules in SecurityConfig (the
authorizeHttpRequests chain handling requestMatchers) to stop permitting these
paths anonymously—either require authentication for "/health" and "/version" or
introduce separate minimal public endpoints (e.g., "/health/public" and
"/version/public") and leave detailed controller methods in HealthController and
VersionController behind authenticated routes; adjust requestMatchers so only
"/user/**" remains permitAll and ensure controllers map detailed endpoints to
the authenticated paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 332dd5f7-fa53-4426-a171-eb876f2293dc
📒 Files selected for processing (6)
pom.xmlsrc/main/java/com/iemr/mmu/controller/health/HealthController.javasrc/main/java/com/iemr/mmu/controller/version/VersionController.javasrc/main/java/com/iemr/mmu/service/health/HealthService.javasrc/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.javasrc/main/java/com/iemr/mmu/utils/mapper/SecurityConfig.java
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | ||
| <maven.test.skip>true</maven.test.skip> | ||
| <build.timestamp>${maven.build.timestamp}</build.timestamp> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expectation:
# - pom.xml enables maven.test.skip
# - PR workflow runs mvn clean install without overriding it
echo "POM test-related flags:"
rg -n '<maven.test.skip>|<testFailureIgnore>' pom.xml
echo
echo "PR build workflow:"
sed -n '18,30p' .github/workflows/build-on-pull-request.ymlRepository: PSMRI/MMU-API
Length of output: 1774
Don't commit maven.test.skip=true to the project defaults.
This skips test compilation and execution for every mvn build command. Any CI pipeline running mvn clean install will bypass tests, masking breaking changes in newly added or modified code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pom.xml` around lines 21 - 25, The pom currently sets the Maven property
maven.test.skip=true which disables test compilation and execution; remove that
property or change it to maven.test.skip=false (or simply delete the
<maven.test.skip> element) so tests run by default, and if skipping is needed in
specific runs use CLI flags (e.g., -DskipTests or -DskipITs) rather than
committing a global default.
| .authorizeHttpRequests(auth -> auth | ||
| .requestMatchers("/user/**").permitAll() | ||
| .requestMatchers("/user/**", "/health", "/version").permitAll() | ||
| .anyRequest().authenticated() |
There was a problem hiding this comment.
Avoid making the detailed health/version payloads anonymous.
src/main/java/com/iemr/mmu/controller/health/HealthController.java now exposes MySQL/Redis component state and timings, and src/main/java/com/iemr/mmu/controller/version/VersionController.java exposes branch/commit metadata. Putting both routes behind permitAll() turns that into unauthenticated reconnaissance. If these endpoints must stay public, keep the public body minimal and gate the detailed view behind auth or a network allowlist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/mmu/utils/mapper/SecurityConfig.java` around lines 41
- 43, SecurityConfig currently permits anonymous access to "/health" and
"/version", exposing detailed payloads from HealthController and
VersionController; update the authorization rules in SecurityConfig (the
authorizeHttpRequests chain handling requestMatchers) to stop permitting these
paths anonymously—either require authentication for "/health" and "/version" or
introduce separate minimal public endpoints (e.g., "/health/public" and
"/version/public") and leave detailed controller methods in HealthController and
VersionController behind authenticated routes; adjust requestMatchers so only
"/user/**" remains permitAll and ensure controllers map detailed endpoints to
the authenticated paths.



📋 Description
JIRA ID:
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
Release Notes
New Features
Chores