prod : yml 분리 #278
prod : yml 분리 #278
Conversation
…ications if not configured
MOSU-267 refactor: 도메인 캐싱 적용
… into feat/mosu-249
… streamline configuration
…ations if not configured
MOSU-249 feat: 배포 환경에 따른 yml 분리
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @polyglot-k, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented a series of changes focused on improving the application's configurability, performance, and user interaction. The core of this pull request involves a significant refactoring of our YAML configuration files to support environment-specific settings more cleanly. Additionally, I've integrated a local caching solution to boost the performance of our notice-related services. Furthermore, I've extended the inquiry management capabilities by adding the ability for users to update their previously submitted inquiries. Minor adjustments were also made to the Discord notification system and a validation annotation to enhance robustness and flexibility.
Highlights
- Configuration Refactoring: I've refactored the application's configuration by splitting the monolithic
application.ymlinto environment-specific files (application-base.yml,application-local.yml,application-prod.yml,application-test.yml). This change significantly improves the clarity and maintainability of our configurations, making it easier to manage settings across different deployment environments. - Local Caching Implementation with Caffeine: I've introduced a local caching mechanism using Caffeine for the
NoticeService. This implementation leveragesCacheGroupandCacheTypeenums to define and manage various cache configurations, applying@Cacheableand@CacheEvictannotations to optimize data retrieval and ensure cache consistency for notices. - Inquiry Update Functionality: I've added new functionality to allow users to update their inquiries. This includes a new
updateInquirymethod inInquiryService, a correspondingPUTendpoint inInquiryController, and a dedicatedInquiryUpdateRequestDTO to handle the request payload. - Discord Notifier Improvement: I've enhanced the
DiscordNotifierto make the Discord webhook URL configurable and optional. The notifier now includes a check to prevent sending notifications if the URL is not provided, which is useful for environments where Discord integration is not active. - Validation Annotation Refinement: I've refined the
PhoneNumberPatternannotation by removing the@NotBlankconstraint. This change allows the phone number field to be optional, providing more flexibility in validation scenarios where a phone number might not always be required.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request refactors the application configuration by splitting the main application.yml into profile-specific files for different environments, which is a great improvement for maintainability. It also introduces Caffeine-based caching for the notice service to enhance performance. My review focuses on potential issues in the new configuration files. I've identified a critical security vulnerability related to exposing all actuator endpoints in production, a high-risk database setting, and a potential configuration error. I've also included a suggestion to improve JPA usage by removing a redundant repository save call.
| web: | ||
| exposure: | ||
| include: "*" |
There was a problem hiding this comment.
Exposing all actuator endpoints via management.endpoints.web.exposure.include: "*" is a critical security vulnerability in a production environment. Endpoints like /env, /heapdump, /shutdown, etc., can leak sensitive information (like configuration properties, passwords) or even allow modification of the application's state. You should explicitly list only the endpoints that are safe and necessary to expose, such as health and info. All exposed endpoints should also be secured.
web:
exposure:
include: health,info
| open-in-view: false | ||
| show-sql: true | ||
| hibernate: | ||
| ddl-auto: update |
There was a problem hiding this comment.
Using spring.jpa.hibernate.ddl-auto: update in a base configuration that might be used by production is very risky. This setting can lead to unintended schema modifications and potential data loss. For production environments, it's much safer to use validate (to verify the schema matches the entities) or none (if you manage schema changes with a tool like Flyway or Liquibase). I recommend changing this to a safer default or overriding it in the production profile.
ddl-auto: validate
| web: | ||
| exposure: | ||
| include: "*" |
There was a problem hiding this comment.
Exposing all actuator endpoints can be a security risk, even in a test environment, if it's accessible from outside your trusted network. It's a good practice to be explicit about which endpoints are exposed to avoid accidentally leaking sensitive information. Consider limiting this to only necessary endpoints like health and info.
web:
exposure:
include: health,info
| hasPermission(inquiry.getUserId(), user); | ||
|
|
||
| inquiry.update(request.title(), request.content(), user.getName()); | ||
| inquiryJpaRepository.save(inquiry); |
There was a problem hiding this comment.
The explicit call to inquiryJpaRepository.save(inquiry) is redundant here. Since the updateInquiry method is annotated with @Transactional, the inquiry entity fetched by getInquiry is in a managed state. Any changes made to it (like in the inquiry.update(...) call) will be automatically detected and persisted to the database when the transaction commits. Removing this line will make the code cleaner and rely on the standard behavior of Spring Data JPA.
| data: | ||
| redis: | ||
| host: ${REDIS_HOST} | ||
| port: ${VELKEY_PORT} |
There was a problem hiding this comment.
The property placeholder for the Redis port is ${VELKEY_PORT}. This seems like a potential typo. Typically, it would be something like ${REDIS_PORT}. Please verify if VELKEY_PORT is the intended environment variable name.
port: ${REDIS_PORT}
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타