[BUGFIX] 소셜 로그인 read-only 트랜잭션 버그 수정 - #96
Merged
Merged
Conversation
p1001q
requested review from
kimyw1018,
on1yoneprivate,
ownue and
rkdehdrbs7885-oss
July 31, 2026 20:50
📝 WalkthroughWalkthrough소셜 로그인 토큰 교환이 클래스 수준의 read-only 트랜잭션을 상속하지 않도록 변경했습니다. 소셜 인증 저장 중 제약조건 위반은 복구 쿼리 없이 도메인 예외로 처리합니다. 동시 요청 테스트와 테스트 import 경로도 수정했습니다. Changes소셜 로그인 트랜잭션 수정
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 1
🧹 Nitpick comments (1)
src/main/java/com/mr/domain/auth/service/AuthTransactionService.java (1)
79-82: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPostgreSQL 중복 키 제약 위반과 실제 원인을 분리하세요.
DataIntegrityViolationException은 이catch에서 항상 요청 유효성 문제가 아닙니다. 이 트랜잭션에는User지연 flush,SocialAuthFK/nullable/check 제약 위반도 포함될 수 있습니다. 현재 코드는 그 모든 원인을INVALID_AUTH_REQUEST로 숨깁니다.
PostgresUniqueConstraintViolationException또는ConstraintViolationExceptionSQL state23505가SocialAuth관련 중복 키 충돌일 때만 매핑하고, 그 외 무결성 violation은 그대로 전파해 서비스 오류 처리가 작동하도록 유지하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/auth/service/AuthTransactionService.java` around lines 79 - 82, Update the DataIntegrityViolationException handling in AuthTransactionService so only SocialAuth-related duplicate-key violations identified as PostgreSQL SQL state 23505 (via PostgresUniqueConstraintViolationException or ConstraintViolationException) map to INVALID_AUTH_REQUEST. Preserve propagation of all other integrity violations, including User flush and SocialAuth foreign-key, nullable, or check constraint failures, so they reach the service error handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/mr/domain/auth/service/AuthTransactionService.java`:
- Around line 79-82: Update completeTokenExchange() to propagate the original
DataIntegrityViolationException instead of converting it to
GeneralException(AuthErrorStatus.INVALID_AUTH_REQUEST), so
AuthService.socialLogin() and socialLoginByCode() can reach
executeSocialLoginForExistingUser(). Preserve the transaction rollback behavior
and apply INVALID_AUTH_REQUEST mapping only after the AuthService recovery path
has been given an opportunity to handle the integrity violation.
---
Nitpick comments:
In `@src/main/java/com/mr/domain/auth/service/AuthTransactionService.java`:
- Around line 79-82: Update the DataIntegrityViolationException handling in
AuthTransactionService so only SocialAuth-related duplicate-key violations
identified as PostgreSQL SQL state 23505 (via
PostgresUniqueConstraintViolationException or ConstraintViolationException) map
to INVALID_AUTH_REQUEST. Preserve propagation of all other integrity violations,
including User flush and SocialAuth foreign-key, nullable, or check constraint
failures, so they reach the service error handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b2dac9d6-5885-4561-8213-223f49e577a5
📒 Files selected for processing (5)
src/main/java/com/mr/domain/auth/service/AuthService.javasrc/main/java/com/mr/domain/auth/service/AuthTransactionService.javasrc/test/java/com/mr/domain/analysis/service/AnalysisRequestFactoryTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisServiceTest.javasrc/test/java/com/mr/domain/playing/service/PlayingServiceTest.java
p1001q
force-pushed
the
fix/#94-auth-transaction-bugs
branch
from
July 31, 2026 21:18
9699d3d to
b18a8aa
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/mr/domain/auth/service/AuthService.java (1)
153-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
exchangeTempCode()의 트랜잭션 경계를 검증하는 통합 회귀 테스트를 추가하세요.현재 주석도
AuthServiceTest가 이 회귀를 잡지 못한다고 명시합니다. 제공된AuthTransactionServiceTest는completeTokenExchange()의 예외 전파만 검증합니다.읽기 전용 트랜잭션 호출자에서 Spring 프록시를 통해
exchangeTempCode()를 호출하세요. 신규 사용자와SocialAuth저장이 성공하는지 검증하세요. 가능하면 운영 DB와 같은 PostgreSQL 환경을 사용하세요. 이 테스트가 없으면NOT_SUPPORTED제거 또는 전파 설정 변경이 다시 배포 장애를 만들 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/auth/service/AuthService.java` around lines 153 - 156, AuthService의 exchangeTempCode() 트랜잭션 경계를 검증하는 통합 회귀 테스트를 추가하세요. readOnly=true 트랜잭션 호출자에서 Spring 프록시를 통해 exchangeTempCode()를 호출하고, 신규 사용자 및 SocialAuth 저장이 성공하는지 검증하세요. 기존 AuthTransactionServiceTest의 completeTokenExchange() 예외 전파 검증과 분리하고, 가능하면 PostgreSQL 기반 테스트 환경을 사용해 `@Transactional`(propagation = Propagation.NOT_SUPPORTED) 설정이 유지되는지 확인하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/com/mr/domain/auth/service/AuthService.java`:
- Around line 153-156: AuthService의 exchangeTempCode() 트랜잭션 경계를 검증하는 통합 회귀 테스트를
추가하세요. readOnly=true 트랜잭션 호출자에서 Spring 프록시를 통해 exchangeTempCode()를 호출하고, 신규 사용자
및 SocialAuth 저장이 성공하는지 검증하세요. 기존 AuthTransactionServiceTest의
completeTokenExchange() 예외 전파 검증과 분리하고, 가능하면 PostgreSQL 기반 테스트 환경을 사용해
`@Transactional`(propagation = Propagation.NOT_SUPPORTED) 설정이 유지되는지 확인하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 95698d65-30c1-42c3-83c3-59aee15b1a13
📒 Files selected for processing (3)
src/main/java/com/mr/domain/auth/service/AuthService.javasrc/main/java/com/mr/domain/auth/service/AuthTransactionService.javasrc/test/java/com/mr/domain/auth/service/AuthTransactionServiceTest.java
on1yoneprivate
approved these changes
Aug 1, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
AuthService.exchangeTempCode()에@Transactional(propagation = Propagation.NOT_SUPPORTED)추가기존에 이 메서드만 트랜잭션 어노테이션 오버라이드가 빠져있어서 클래스 레벨의
@Transactional(readOnly = true)를 그대로 물려받고 있었습니다. 그 결과POST /api/auth/token/exchange(웹 기반 카카오/구글 로그인의 핵심 API) 호출 시 신규 유저 INSERT를 시도하다가
ERROR: cannot execute INSERT in a read-only transaction로 매번 실패했습니다.실제 배포 서버에서 로그인 테스트 중 발견했고, 서버 로그로 원인을 확정했습니다.
AuthTransactionService.completeTokenExchange()의 동시성 복구 로직 수정SocialAuthINSERT가 유니크 제약 위반으로 실패하면, PostgreSQL은 트랜잭션 전체를 abort 상태로만듭니다. 기존 코드는 catch 블록에서 같은(이미 abort된) 트랜잭션 안에 복구 쿼리를 다시 시도해서,
동시에 같은 소셜 계정으로 신규 로그인이 두 번 들어오면 복구 자체가 실패하고 500이 발생했습니다.
처음엔
executeLinkSocialAccount()처럼 곧바로 도메인 예외를 던지는 방식으로 정리했는데, CodeRabbit리뷰에서 그렇게 하면
AuthService.socialLogin()/socialLoginByCode()가 기대하는DataIntegrityViolationExceptioncatch(동시 로그인 재시도 복구 경로,executeSocialLoginForExistingUser())에안 걸린다는 지적이 있었고 확인해보니 맞아서, 최종적으로는 catch 자체를 없애고 원본 예외를 그대로
전파하도록 수정했습니다. 대신 복구 경로가 없는
exchangeTempCode()쪽에서만 별도로DataIntegrityViolationException→INVALID_AUTH_REQUEST매핑을 유지했습니다.backingTrack→backingtrack패키지명 변경 시 누락된 import 경로 3개 수정PlayingServiceTest,AnalysisRequestFactoryTest,AnalysisServiceTest가 여전히 옛날 대문자패키지 경로를 참조하고 있어서
develop자체의 테스트 컴파일이 깨져있던 상태였습니다(배포 파이프라인이테스트를 건너뛰어서(
-x test) 아무도 못 보고 있었음). import 경로만 정정했습니다.🔥 리뷰 요청 사항
POST /api/auth/login/{socialType})에는 영향 없는지completeTokenExchange()예외 처리 변경이 기존 흐름과 일관되는지✅ 체크리스트
📎 참고 사항
PlayingServiceTest에서 이번 수정과 무관한 실패 2건을발견했습니다(
PlayingService.startPlaying()의 백킹트랙 조회 로직과 테스트 mock 불일치로 추정).이 PR 범위 밖이라 손대지 않았고, 고원정 님의 별도 확인이 필요합니다~
exchangeTempCode()의 read-only 트랜잭션 상속 버그(이번 PR의핵심 수정 사항)를 재현하는 실제 DB 통합 테스트를 추가하려고 했는데, 기존
AuthServiceTest가클래스 레벨
@Transactional로 감싸져 있어서 그 안에서는 이 회귀가 애초에 재현이 안 됩니다(테스트가 이미 자기 트랜잭션을 열어놓은 상태라, 그 안에서 호출되는 메서드의
readOnly/propagation설정이 실제 커넥션에 반영되지 않음 — 이 버그가 로컬 테스트로는 못 잡히고배포 서버에서만 터졌던 이유이기도 합니다). 제대로 잡으려면 별도 테스트 클래스(+ DB 수동 정리)가
필요해서 비용 대비 지금 우선순위는 낮다고 보고, 대신 해당 메서드에 왜 이 어노테이션을 지우면
안 되는지 경고 주석만 남겨뒀습니다. 나중에 유사 버그가 또 나오면 그때 별도로 다루는 게 좋을 것 같아요.
Summary by CodeRabbit
버그 수정
테스트