Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes introduce a "swagger" Spring profile to enable automated API documentation generation in CI/CD pipelines. They exclude Redis and authentication-dependent beans when active, add in-memory H2 databases, provide new configuration profiles, and establish a GitHub Actions workflow that builds the API, extracts Swagger JSON, and creates pull requests in the AMRIT-Docs repository. Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant API as Helpline104 API
participant Swagger as Swagger Generator
participant Docs as AMRIT-Docs Repo
GHA->>API: Checkout API repository
GHA->>API: Build with Maven (swagger profile)
GHA->>API: Start API on port 9090
API->>Swagger: Initialize with H2 databases
GHA->>Swagger: Poll for /v3/api-docs
Swagger-->>GHA: Return Swagger JSON
GHA->>GHA: Validate & parse to helpline104-api.json
GHA->>API: Stop API
GHA->>Docs: Checkout AMRIT-Docs repository
GHA->>Docs: Copy Swagger JSON to docs/swagger/
GHA->>Docs: Create pull request with auto/swagger-update branch
Docs-->>GHA: PR created successfully
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/swagger-json.yml:
- Around line 8-11: The workflow job "swagger-sync" lacks a Redis service
required by application-swagger.properties (which sets
spring.redis.host=localhost); either add a Redis service entry to the
swagger-sync job so the API can connect to Redis during startup, or update
application-swagger.properties to disable Redis for Swagger generation (e.g.,
remove or override spring.redis.* settings) so the job doesn't require Redis;
modify the swagger-sync job configuration to reference the Redis service name
and ensure the service is available before API startup.
In `@src/main/java/com/iemr/helpline104/config/RedisConfig.java`:
- Around line 38-46: The new RedisConfig.genericRedisTemplate bean conflicts
with App.redisTemplate and causes ambiguous injection; update TokenDenylist and
JwtAuthenticationUtil to inject a specific bean by adding
`@Qualifier`("genericRedisTemplate") (or `@Qualifier`("redisTemplate") if you want
the App.redisTemplate) to their RedisTemplate<String, Object> injection points,
and change RedisConfig.genericRedisTemplate to use new
GenericJackson2JsonRedisSerializer() instead of
Jackson2JsonRedisSerializer<>(Object.class) so type metadata is preserved; also
reconcile duplicate beans by removing or consolidating RedisConfig.redisTemplate
or App.redisTemplate to keep a single authoritative RedisTemplate definition.
In `@src/main/resources/application-swagger.properties`:
- Line 18: The jwt.secret property is hardcoded to the literal "JWT_SECRET";
change it to read from an environment variable instead (e.g., replace the value
for jwt.secret with a reference to an env var such as JWT_SECRET_KEY or a Spring
placeholder like ${JWT_SECRET_KEY}) so the secret is not baked into
application-swagger.properties; update any related docs or README to require
JWT_SECRET_KEY in the environment and ensure the code that loads jwt.secret
(property key jwt.secret) continues to read the property.
In `@src/main/resources/application.properties`:
- Around line 36-37: The spring.datasource.username and
spring.datasource.password entries currently set H2 defaults (username=SA, empty
password) which will override MySQL credentials; remove these two properties
from the main application.properties (spring.datasource.username and
spring.datasource.password) or replace them with environment-variable
placeholders (e.g., spring.datasource.username=${DB_USERNAME:} and
spring.datasource.password=${DB_PASSWORD:}) so production/MySQL profiles supply
real credentials, and leave the H2 values only in
application-swagger.properties.
🧹 Nitpick comments (3)
pom.xml (1)
252-256: Consider limiting H2 to test scope if only used for Swagger profile.The H2 dependency is added with
runtimescope, which means it will be packaged in the production WAR. If H2 is only intended for the Swagger documentation workflow (usingapplication-swagger.properties), consider usingtestscope instead, or ensure the H2 console is disabled in production configurations to avoid potential security exposure.If the Swagger profile is only run in CI (as the workflow suggests),
testscope might suffice:♻️ Optional: Change to test scope
<dependency> <groupId>com.h2database</groupId> <artifactId>h2</artifactId> - <scope>runtime</scope> + <scope>test</scope> </dependency>Note: If
testscope doesn't work withmvn spring-boot:run, keepruntimebut ensurespring.h2.console.enabled=falseis set for non-swagger profiles.src/main/resources/application-swagger.properties (1)
8-10: Comment is misleading - Redis is configured, not disabled.The comment suggests Redis can be disabled, but the configuration actually enables Redis with localhost settings. If Redis is truly optional for Swagger docs generation, consider actually disabling it or clarifying the comment.
📝 Option A: Actually disable Redis for Swagger
-# Disable Redis if not needed for docs (optional) -spring.redis.host=localhost -spring.redis.port=6379 +# Disable Redis for Swagger docs +spring.data.redis.enabled=false +spring.session.store-type=none📝 Option B: Clarify the comment
-# Disable Redis if not needed for docs (optional) +# Redis configuration for local/CI Swagger environment spring.redis.host=localhost spring.redis.port=6379.github/workflows/swagger-json.yml (1)
3-6: Consider adding concurrency control to prevent parallel runs.The workflow runs on every push to
main, which could result in multiple simultaneous workflow runs creating conflicting PRs if pushes happen in quick succession.♻️ Add concurrency group
on: push: branches: [ main ] workflow_dispatch: +concurrency: + group: swagger-sync + cancel-in-progress: true + jobs:
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/swagger-json.yml:
- Line 44: The workflow sets the Spring Boot arg --spring.redis.host=redis which
will not resolve when the job runs on ubuntu-latest; update the argument in the
YAML entry that contains -Dspring-boot.run.arguments (the string including
--server.port=9090,--spring.redis.host=redis) to use
--spring.redis.host=localhost so Redis is reached via the mapped localhost:6379
port.
🧹 Nitpick comments (1)
.github/workflows/swagger-json.yml (1)
88-99: Consider addingdelete-branch: truefor automatic cleanup.After the PR is merged (or closed), the auto-generated branch
auto/swagger-update-${{ github.run_id }}will remain in the AMRIT-Docs repository. Addingdelete-branch: trueto the action configuration would clean up stale branches automatically.♻️ Proposed enhancement
- name: Create Pull Request uses: peter-evans/create-pull-request@v6 with: token: ${{ secrets.DOCS_REPO_TOKEN }} path: amrit-docs branch: auto/swagger-update-${{ github.run_id }} base: main commit-message: Auto-update Helpline104-API swagger title: Auto-update Helpline104-API swagger + delete-branch: true body: | This PR automatically updates the Helpline104-API Swagger JSON from the latest main branch build.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/iemr/helpline104/utils/JwtAuthenticationUtil.java`:
- Around line 29-31: The redisTemplate field in JwtAuthenticationUtil is
autowired with the genericRedisTemplate (RedisTemplate<String, Object>) which
deserializes to LinkedHashMap and causes a ClassCastException when cast to
M_User; change the injection to use the dedicated RedisTemplate<String, M_User>
bean configured in RedisConfig by removing the
`@Qualifier`("genericRedisTemplate") (or replacing it with the M_User-typed bean
name) so the field type matches RedisTemplate<String, M_User> and uses
Jackson2JsonRedisSerializer<M_User> for correct deserialization when
JwtAuthenticationUtil accesses/stores M_User objects.
🧹 Nitpick comments (1)
src/main/java/com/iemr/helpline104/utils/JwtAuthenticationUtil.java (1)
36-39: Consider consistent dependency injection approach.The constructor initializes
cookieUtilandjwtUtil, whileredisTemplateandiEMRUserRepositoryCustomuse field injection. This mixed approach can complicate testing and makes dependencies less explicit.♻️ Suggested refactor for constructor injection
- `@Autowired` - private CookieUtil cookieUtil; - `@Autowired` - private JwtUtil jwtUtil; - `@Autowired` - `@Qualifier`("genericRedisTemplate") - private RedisTemplate<String, Object> redisTemplate; - `@Autowired` - private IEMRUserRepositoryCustom iEMRUserRepositoryCustom; + private final CookieUtil cookieUtil; + private final JwtUtil jwtUtil; + private final RedisTemplate<String, Object> redisTemplate; + private final IEMRUserRepositoryCustom iEMRUserRepositoryCustom; private final Logger logger = LoggerFactory.getLogger(this.getClass().getName()); - public JwtAuthenticationUtil(CookieUtil cookieUtil, JwtUtil jwtUtil) { + public JwtAuthenticationUtil( + CookieUtil cookieUtil, + JwtUtil jwtUtil, + `@Qualifier`("genericRedisTemplate") RedisTemplate<String, Object> redisTemplate, + IEMRUserRepositoryCustom iEMRUserRepositoryCustom) { this.cookieUtil = cookieUtil; this.jwtUtil = jwtUtil; + this.redisTemplate = redisTemplate; + this.iEMRUserRepositoryCustom = iEMRUserRepositoryCustom; }
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/main/environment/104_swagger.properties`:
- Line 26: Replace the hardcoded JWT secret in 104_swagger.properties by
switching the property jwt.secret from a literal value to an environment
placeholder (e.g., ${JWT_SECRET_KEY}); update any related config or docs that
reference the swagger-only property so JwtUtil (and any code that reads
jwt.secret) will pick up the environment variable instead of a committed secret,
and ensure the application.yml/bootstrap config supports resolving
${JWT_SECRET_KEY} at runtime.
In `@src/main/java/com/iemr/helpline104/config/SwaggerConfig.java`:
- Around line 22-31: Update the OpenAPI Info in the SwaggerConfig: replace the
placeholder .version("version") with the real API version (e.g., "1.0.0" or load
from an environment property/pom property) and change the
Info().description(...) text to accurately describe the Helpline 104 service
instead of "creation and management of beneficiaries"; modify the OpenAPI
builder call that constructs new OpenAPI().info(new
Info().title(...).version(...).description(...)) so it reads the correct version
source and a correct Helpline 104 description.
In `@src/main/resources/application-swagger.properties`:
- Around line 11-12: The Hibernate dialect is set to MySQL while the datasource
is H2; update the two properties spring.jpa.database-platform and
spring.jpa.properties.hibernate.dialect to use org.hibernate.dialect.H2Dialect
so the dialect matches the in-memory H2 DB (modify the entries currently set to
org.hibernate.dialect.MySQLDialect).
- Line 43: Replace the SpEL fallback in the jwt.secret property which won't be
evaluated; update the jwt.secret declaration (currently using
`${JWT_SECRET_KEY:#{T(java.util.UUID).randomUUID().toString()}}`) to use a plain
placeholder fallback consistent with other env files (e.g., `${JWT_SECRET_KEY}`
or a simple default literal) so it no longer contains SpEL; edit the property in
application-swagger.properties ensuring the key name `jwt.secret` remains
unchanged.
🧹 Nitpick comments (3)
src/main/environment/104_swagger.properties (1)
22-23: Redis config is unnecessary under the swagger profile.
RedisConfig,RedisStorage, andSessionObjectare all gated with@Profile("!swagger"), sospring.redis.hostwon't be used. Consider removing it to avoid confusion, or add a comment clarifying it's inert.src/main/java/com/iemr/helpline104/config/SwaggerConfig.java (1)
27-31: Prefer importingServerinstead of using fully-qualified class names inline.The
io.swagger.v3.oas.models.servers.Serveris repeated three times inline. A simple import would improve readability.♻️ Proposed refactor
Add to imports:
import io.swagger.v3.oas.models.servers.Server;Then use:
- .servers(java.util.Arrays.asList( - new io.swagger.v3.oas.models.servers.Server().url(devUrl).description("Dev"), - new io.swagger.v3.oas.models.servers.Server().url(uatUrl).description("UAT"), - new io.swagger.v3.oas.models.servers.Server().url(demoUrl).description("Demo") + .servers(java.util.Arrays.asList( + new Server().url(devUrl).description("Dev"), + new Server().url(uatUrl).description("UAT"), + new Server().url(demoUrl).description("Demo") ));src/main/resources/application-swagger.properties (1)
29-33: Redis properties are configured but Redis auto-configuration is excluded.Lines 31-33 set Redis connection details, but line 16 excludes
RedisAutoConfiguration. These properties are effectively dead configuration. This is harmless but potentially confusing for future maintainers — consider removing them or adding a comment explaining they're present only as fallbacks.
|
2 participants
Summary by CodeRabbit
New Features
Chores