[배포] DataIntegrityViolationException 처리#315
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 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 @wlgns12370, 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!
이 PR은 주로 OAuth 사용자 처리 중 DataIntegrityViolationException에 대한 오류 처리를 개선하는 데 중점을 둡니다. 또한, DepositStatus 열거형을 문자열로 저장하기 위한 데이터베이스 스키마 마이그레이션 및 해당 엔티티 업데이트, 그리고 사용자 테스트 픽스처의 사소한 정리를 포함합니다.
Highlights
- 오류 처리 개선: OAuthUserPersistenceProcessor에서 DataIntegrityViolationException을 처리하고 USER_ALREADY_EXISTS 오류 코드를 가진 CustomRuntimeException을 발생시키도록 구현했습니다.
- 데이터베이스 스키마 마이그레이션: virtual_account_log 테이블의 deposit_status 컬럼을 VARCHAR 타입으로 변경하는 새로운 Flyway 마이그레이션 스크립트를 추가했습니다.
- 엔티티 업데이트: VirtualAccountLogJpaEntity의 depositStatus 필드에 @Enumerated(EnumType.STRING) 어노테이션을 적용하여 열거형 값을 문자열로 영속화하도록 했습니다.
- 테스트 픽스처 정리: 사용자 테스트 픽스처에서 더 이상 필요 없는 agreedToTermsOfService 및 agreedToPrivacyPolicy 필드를 제거했습니다.
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
안녕하세요. DataIntegrityViolationException 처리를 추가하고 deposit_status 필드 타입을 개선하는 PR을 리뷰했습니다. OAuth 사용자 생성 시 발생할 수 있는 경쟁 상태(race condition)를 처리한 점과, enum 타입을 데이터베이스에 문자열로 저장하도록 변경한 점은 좋은 개선입니다. 다만, 리뷰 과정에서 몇 가지 잠재적인 위험 요소를 발견했습니다. 특히 데이터베이스 마이그레이션 스크립트는 데이터 손실을 유발할 수 있어 주의가 필요합니다. 자세한 내용은 아래 주석을 확인해주세요.
| ALTER TABLE virtual_account_log | ||
| DROP COLUMN deposit_status; | ||
|
|
||
| ALTER TABLE virtual_account_log | ||
| ADD deposit_status VARCHAR(255) NULL; No newline at end of file |
There was a problem hiding this comment.
현재 마이그레이션 스크립트는 deposit_status 컬럼을 삭제 후 재생성하므로, 기존의 모든 데이터가 손실될 위험이 있습니다. 또한, 컬럼이 NULL을 허용하도록 재생성되어 기존 행들의 해당 컬럼 값은 NULL이 됩니다. VirtualAccountLogJpaEntity에서는 이 필드가 DepositStatus.WAITING으로 초기화되므로, NULL 값은 NullPointerException 등 예기치 않은 동작을 유발할 수 있습니다.
데이터 손실을 방지하고 데이터 무결성을 지키기 위해, ALTER COLUMN을 사용해 컬럼 타입을 변경하거나, 데이터를 임시 컬럼으로 백업 후 복원하는 방식으로 마이그레이션 스크립트를 수정하는 것을 강력히 권장합니다.
| return userRepository.save(newUser); | ||
| }); | ||
| try { | ||
| return userRepository.findByPhoneNumber(info.phoneNumber()) |
There was a problem hiding this comment.
전화번호로 기존 사용자를 찾는 로직에 버그가 있을 수 있습니다. UserJpaEntity에는 PhoneNumberUtil.formatPhoneNumber()를 통해 'U' 접두사가 붙은 전화번호가 저장됩니다. 하지만 현재 코드는 접두사 없는 원본 전화번호로 조회하고 있어, 항상 기존 사용자를 찾지 못하고 신규 사용자 생성을 시도하게 됩니다. 이로 인해 의도치 않게 DataIntegrityViolationException이 발생하고 예외 처리 로직에 의존하게 될 수 있습니다.
findByPhoneNumber 호출 전에 전화번호를 포맷팅하여 이 문제를 해결하는 것이 좋습니다.
| return userRepository.findByPhoneNumber(info.phoneNumber()) | |
| return userRepository.findByPhoneNumber(PhoneNumberUtil.formatPhoneNumber(info.phoneNumber())) |
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타