prod : env 에 따른 cookie 분리 및 알림톡 전송 시 전화번호 정규화 오류 해결 #326
Conversation
|
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/Issue comments)Type 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!
이 풀 리퀘스트는 프로덕션 환경에 따른 쿠키 분리 로직을 개선하고, 알림톡 전송 시 발생하던 전화번호 정규화 오류를 해결합니다. 또한, 환경별 설정을 보다 유연하게 관리하기 위한 프로파일 유틸리티를 도입하고, 애플리케이션 시작 시 활성 프로파일을 명확히 로깅하도록 업데이트되었습니다.
Highlights
- 환경별 쿠키 처리 개선: 환경에 따라 쿠키의
secure속성을 동적으로 설정하도록CookieBuilderUtil이 수정되었습니다. 이는 'prod' 프로파일이 활성 상태일 때 쿠키가 HTTPS를 통해서만 전송되도록 보장합니다. - 알림톡 전화번호 정규화 오류 수정:
NotifyService에서 전화번호 정규화 로직이 수정되어,replaceFirst("^.", "")호출이 제거되었습니다. 이 변경은 알림톡 전송 시 발생할 수 있는 전화번호 형식 오류를 해결합니다. - Spring 프로파일 유틸리티 추가: 새로운
ProfileUtil유틸리티 클래스가 추가되어 애플리케이션의 활성 Spring 프로파일을 프로그램적으로 확인할 수 있게 되었습니다. 이는 환경별 로직 구현에 활용됩니다. - 애플리케이션 시작 시 프로파일 로깅:
MosuServerApplication시작 시 활성 프로파일을 로깅하는 기능이 추가되어, 애플리케이션이 올바른 환경에서 실행되고 있는지 쉽게 확인할 수 있습니다.
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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
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
안녕하세요. PR을 리뷰했습니다. 전반적으로 환경에 따라 쿠키 설정을 분리하고 전화번호 정규화 오류를 해결하는 변경 사항들이 잘 적용되었습니다. 몇 가지 개선점과 잠재적인 문제를 발견하여 코멘트를 남깁니다. 주요 내용은 다음과 같습니다:
CookieBuilderUtil에서secure속성 설정 로직의 불일치 문제ProfileUtil의 구현을 더 간결하게 개선하고, 장기적인 관점에서 설계 개선 제안MosuServerApplication에 추가된 디버깅용 로그 정리 제안
자세한 내용은 각 파일의 코멘트를 확인해주세요.
| boolean isProd = ProfileUtil.isProfileActiveStatic("prod"); | ||
|
|
||
| return createBaseResponseCookieBuilder(name, value, maxAge) | ||
| .secure(true) | ||
| .secure(isProd) | ||
| .domain(".mosuedu.com") | ||
| .sameSite("Strict") | ||
| .build(); |
There was a problem hiding this comment.
안녕하세요. prod 프로필 여부에 따라 쿠키의 secure 속성을 동적으로 설정하도록 변경하신 부분 잘 보았습니다. 하지만 createDevelopCookie 메소드(86라인)에서는 여전히 secure 속성을 true로 하드코딩하고 있어 createDevelopResponseCookie 메소드와 동작이 일치하지 않습니다. 두 메소드의 동작을 일관성 있게 유지하기 위해 createDevelopCookie 메소드도 동일한 로직을 적용하도록 수정하는 것을 고려해 보시는 것이 좋겠습니다. 이 불일치는 예기치 않은 쿠키 관련 문제를 일으킬 수 있습니다.
| log.info("Active Profiles: {}", (Object) ProfileUtil.getActiveProfilesStatic()); | ||
| log.info("profile : {}", ProfileUtil.isProfileActiveStatic("prod")); | ||
| log.info("profile : {}", ProfileUtil.isProfileActiveStatic("test")); |
There was a problem hiding this comment.
안녕하세요. 애플리케이션 시작 시 활성 프로필을 확인하기 위한 로그를 추가하신 것으로 보입니다. 이 로그는 개발 및 디버깅 과정에서는 유용하지만, 프로덕션 환경에서는 불필요한 정보가 될 수 있습니다. log.debug 레벨로 변경하여 개발 환경에서만 확인하고 프로덕션 로그에서는 제외하는 것을 권장합니다.
| log.info("Active Profiles: {}", (Object) ProfileUtil.getActiveProfilesStatic()); | |
| log.info("profile : {}", ProfileUtil.isProfileActiveStatic("prod")); | |
| log.info("profile : {}", ProfileUtil.isProfileActiveStatic("test")); | |
| log.debug("Active Profiles: {}", (Object) ProfileUtil.getActiveProfilesStatic()); | |
| log.debug("profile 'prod' is active: {}", ProfileUtil.isProfileActiveStatic("prod")); | |
| log.debug("profile 'test' is active: {}", ProfileUtil.isProfileActiveStatic("test")); |
| public static boolean isProfileActiveStatic(String profile) { | ||
| for (String active : environment.getActiveProfiles()) { | ||
| if (active.equals(profile)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
isProfileActiveStatic 메소드의 구현을 Java Stream API를 사용하여 더 간결하게 개선할 수 있습니다. 가독성을 높이고 코드를 더 현대적인 스타일로 유지하는 데 도움이 됩니다.
public static boolean isProfileActiveStatic(String profile) {
return java.util.Arrays.stream(environment.getActiveProfiles())
.anyMatch(profile::equals);
}| public class ProfileUtil implements ApplicationContextAware { | ||
|
|
||
| private static Environment environment; | ||
|
|
||
| public static String[] getActiveProfilesStatic() { | ||
| return environment.getActiveProfiles(); | ||
| } | ||
|
|
||
| public static boolean isProfileActiveStatic(String profile) { | ||
| for (String active : environment.getActiveProfiles()) { | ||
| if (active.equals(profile)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { | ||
| environment = applicationContext.getEnvironment(); | ||
| } | ||
| } |
There was a problem hiding this comment.
ApplicationContextAware와 static 필드를 사용하여 Environment에 접근하는 방식은 동작은 하지만, 몇 가지 설계적인 단점이 있습니다.
- 테스트의 어려움:
static필드에 의존하는 코드는 단위 테스트하기가 더 어렵습니다.Environment를 모의(mock) 객체로 대체하기가 까다롭습니다. - 강한 결합도:
ProfileUtil이 스프링 컨텍스트에 강하게 결합됩니다.
더 나은 대안으로, ProfileUtil을 일반적인 빈(bean)으로 만들고 Environment를 생성자를 통해 주입받는 것을 고려해볼 수 있습니다. 이 유틸리티를 사용하는 CookieBuilderUtil 같은 클래스도 @UtilityClass 대신 빈으로 만들고 ProfileUtil을 주입받도록 리팩토링하면 전체적인 설계가 더 깔끔해지고 테스트하기 쉬워질 것입니다.
물론 현재 구조에서 @UtilityClass를 유지해야 한다면 지금의 구현이 현실적인 해결책일 수 있습니다. 장기적인 관점에서 리팩토링을 고려해보시면 좋을 것 같습니다.
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타