refactor: SignUpToken 중복 코드 제거, 회원가입 로직 통합 #445
Conversation
- SignUpService의 의존성이 변하자, OAuthSignUpService와 EmailSignUpService의 코드가 변했다 -> DIP 위반
nayonsoso
requested review from
Gyuhyeok99,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
✨ 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 (
|
nayonsoso
changed the title
nayonsoso
force-pushed
the
refactor/444-sign-up-token
branch
from
971dc60 to
77b3d4f
Compare
nayonsoso
changed the title
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
고생하셨습니다 ! 크게 문제가 되는 부분은 없는 거 같습니다.
| return ResponseEntity.ok(signInResponse); | ||
| } | ||
| SignInResponse signInResponse = oAuthSignUpService.signUp(signUpRequest); | ||
| SignInResponse signInResponse = signUpService.signUp(signUpRequest); |
There was a problem hiding this comment.
signUp 관련된 로직이 한 곳으로 통합되니까 정말 깔끔하네요 ....!!!!
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 코드가 엄청 깔끔해졌네요!
궁금한 의견만 몇 개 남겨놓았습니다!
| public void saveInterestedCountry(SiteUser siteUser, List<String> koreanNames) { | ||
| List<InterestedCountry> interestedCountries = countryRepository.findAllByKoreanNameIn(koreanNames) | ||
| .stream() | ||
| .map(country -> new InterestedCountry(siteUser, country)) | ||
| .toList(); | ||
| interestedCountryRepository.saveAll(interestedCountries); |
There was a problem hiding this comment.
@transactional을 안붙이신 이유가 혹시 있나요!?
saveAll하다가 일부만 저장될 수도 있을 거 같아서요!
There was a problem hiding this comment.
동의합니다! 제가 실수로 누락했습니다😓
꼼꼼히 봐주셔서 감사합니다🙇🏻♀️
d37fb7d
| private final RegionRepository regionRepository; | ||
| private final InterestedRegionRepository interestedRegionRepository; | ||
|
|
||
| public void saveInterestedRegion(SiteUser siteUser, List<String> koreanNames) { |
| public Optional<String> findByEmail(String email) { | ||
| String encodedPassword = redisTemplate.opsForValue().getAndDelete(convertToKey(email)); |
There was a problem hiding this comment.
조회와 동시에 삭제하는 것보다는 delete함수 따로 만들어서
@Transactional
public SignInResponse signUp(SignUpRequest signUpRequest) {
// ... 기존 로직 ...
// 임시 저장된 비밀번호 가져오기
String password = getTemporarySavedPassword(email, authType);
// 사용자 저장
SiteUser siteUser = siteUserRepository.save(new SiteUser(...));
// 관심 지역, 국가 저장
interestedRegionService.saveInterestedRegion(siteUser, signUpRequest.interestedRegions());
interestedCountryService.saveInterestedCountry(siteUser, signUpRequest.interestedCountries());
// 회원가입 완료되면 임시 비밀번호 삭제
deleteTemporarySavedPassword(email, authType);
// 로그인
return signInService.signIn(siteUser);
}
이런 건 별로인가요? 이러면 중간에 실패하면 임시 비밀번호로 재시도 가능할 거 같다는 생각이 들었는데 혹시 제가 놓친 게 있을까요
There was a problem hiding this comment.
너무 좋은 의견입니다🥹
규혁님이 주신 의견에서 생각난건데, 회원가입 완료 이후 SignUpToken도 삭제하는 로직을 추가 해봤어요..!
이렇게 바꿔봤는데 어떻게 생각하시나요?
e9a79df
- '회원가입' 서비스에 있는게 더 적절할 것이라 판단
![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
♻️ Duplicate comments (4)
src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java (1)
80-83: 새로운 삭제 메서드가 추가되었습니다 👍회원가입 완료 후 토큰을 정리하는
deleteByEmail메서드 추가가 좋네요! 이전 리뷰 코멘트에서 제안된 내용이 잘 반영되었습니다.src/main/java/com/example/solidconnection/auth/service/PasswordTemporaryStorage.java (2)
30-36:findByEmail메서드 구현이 깔끔합니다Optional을 활용한 null 처리가 적절하게 구현되었네요. 다만 이전 리뷰 코멘트에서 언급된 것처럼, 조회와 삭제를 분리한 현재 구조가 중간 실패 시 재시도를 가능하게 해주는 좋은 설계입니다.
38-41: 삭제 메서드가 적절히 구현되었습니다이전 리뷰어의 제안대로 별도의 delete 메서드를 만들어 회원가입 프로세스 중간에 실패하면 임시 비밀번호로 재시도가 가능하도록 구현되었네요!
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (1)
95-100: 회원가입 데이터 정리 로직이 잘 구현되었습니다이전 리뷰 코멘트에서 제안된 대로 회원가입 완료 후 SignUpToken과 임시 비밀번호를 모두 삭제하는 로직이 잘 구현되었네요! AuthType에 따라 적절히 분기 처리하여 정리하는 구조가 깔끔합니다.
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java (2)
25-25: 상수의 접근 제어자가 불필요하게 private으로 변경되었습니다
AUTH_TYPE_CLAIM_KEY는 토큰 파싱 시 사용되는 핵심 상수인데, 접근 제어자가 package-private에서 private으로 변경되었네요. 현재는 같은 클래스 내에서만 사용되고 있지만, 향후 테스트나 관련 클래스에서 접근이 필요할 수 있으니 package-private으로 유지하는 것이 좋을 것 같습니다.
52-52: 인라인 주석이 메서드 시그니처에 추가되었습니다메서드 시그니처 라인에 주석을 추가하기보다는 JavaDoc이나 별도 라인으로 분리하는 것이 가독성에 더 좋을 것 같아요.
-private void validateFormatAndExpiration(String token) { // 파싱되는지, AuthType이 포함되어있는지 검증 +// 파싱되는지, AuthType이 포함되어있는지 검증 +private void validateFormatAndExpiration(String token) {src/main/java/com/example/solidconnection/auth/service/PasswordTemporaryStorage.java (1)
15-15: Redis 키 프리픽스를 더 구체적으로 변경하는 것을 고려해보세요현재
KEY_PREFIX가"password:"로 되어 있는데, 이는 너무 일반적인 이름이에요. 다른 기능에서도 비밀번호 관련 키를 사용할 수 있으니"signup:password:"같은 더 구체적인 네이밍을 고려해보시면 어떨까요?-private static final String KEY_PREFIX = "password:"; +private static final String KEY_PREFIX = "signup:password:";src/main/java/com/example/solidconnection/auth/service/SignUpService.java (1)
87-93: 비밀번호 조회 시 에러 처리를 개선할 수 있을 것 같습니다EMAIL 타입일 때 임시 저장소에서 비밀번호를 찾지 못하면
SIGN_UP_TOKEN_INVALID예외를 던지는데, 이는 토큰 자체의 문제가 아니라 임시 데이터 문제일 수 있어요. 더 구체적인 에러 코드를 사용하면 디버깅에 도움이 될 것 같습니다.private String getTemporarySavedPassword(String email, AuthType authType) { if (authType == AuthType.EMAIL) { return passwordTemporaryStorage.findByEmail(email) - .orElseThrow(() -> new CustomException(SIGN_UP_TOKEN_INVALID)); + .orElseThrow(() -> new CustomException(TEMPORARY_PASSWORD_NOT_FOUND)); } return null; }새로운 에러 코드
TEMPORARY_PASSWORD_NOT_FOUND를 ErrorCode enum에 추가하는 것도 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/PasswordTemporaryStorage.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/SignUpService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java(4 hunks)src/main/java/com/example/solidconnection/location/country/service/InterestedCountryService.java(1 hunks)src/main/java/com/example/solidconnection/location/region/service/InterestedRegionService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/solidconnection/location/country/service/InterestedCountryService.java
- src/main/java/com/example/solidconnection/location/region/service/InterestedRegionService.java
- src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/example/solidconnection/auth/service/PasswordTemporaryStorage.java (2)
src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java (1)
Component(12-32)src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java (1)
Component(21-84)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (3)
src/main/java/com/example/solidconnection/location/country/service/InterestedCountryService.java (1)
Service(12-27)src/main/java/com/example/solidconnection/location/region/service/InterestedRegionService.java (1)
Service(12-27)src/main/java/com/example/solidconnection/auth/service/oauth/OAuthService.java (1)
Service(24-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (2)
27-29: 클래스가 추상 클래스에서 구체 클래스로 변경되었습니다기존의 추상 클래스 구조에서 단일 구체 클래스로 통합된 것이 응집도를 높이는 좋은 리팩토링이네요!
42-46: 토큰 검증과 파싱이 일관되게 처리됩니다SignUpTokenProvider를 통해 토큰 검증, 이메일 추출, AuthType 파싱이 통합되어 처리되는 구조가 깔끔합니다.
| // 회원가입을 위해 저장한 데이터(SignUpToken, 비밀번호) 삭제 | ||
| clearSignUpData(email, authType); | ||
|
|
||
| return response; |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
트랜잭션 실패 시 데이터 정리에 대한 고려가 필요할 수 있습니다
현재 구조에서는 clearSignUpData가 트랜잭션 내부에서 실행되는데, 만약 이 메서드 실행 후 트랜잭션이 롤백되면 Redis의 데이터는 이미 삭제되어 재시도가 불가능할 수 있어요. 이를 방지하기 위해 트랜잭션 커밋 후에 실행되도록 하는 것을 고려해보세요.
🏁 Script executed:
#!/bin/bash
# TransactionalEventListener나 @AfterCommit 같은 트랜잭션 후처리 메커니즘 사용 확인
rg -A 5 "TransactionalEventListener|AfterCommit|TransactionSynchronization" --type javaLength of output: 89
트랜잭션 커밋 후 Redis 데이터 정리 고려 필요
- 문제 요약
clearSignUpData가 서비스 레이어 트랜잭션 내부에서 실행되면, 트랜잭션이 롤백될 경우 Redis에 삭제된 회원가입 토큰 및 비밀번호를 복구할 수 없어 재시도가 불가능해질 수 있습니다. - 해결 방안
- @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) 또는 TransactionSynchronizationManager.registerSynchronization(...)를 활용하여 트랜잭션 커밋 직후에 clearSignUpData가 실행되도록 분리해주세요.
- 확인 요청
rg -A5 'TransactionalEventListener|AfterCommit|TransactionSynchronization'명령 실행 결과 관련 애너테이션/메커니즘이 발견되지 않았습니다. 적용 여부를 재확인하고, 미적용 시 위 방안을 검토 부탁드립니다.
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/auth/service/SignUpService.java
around lines 69 to 72, clearSignUpData(email, authType) is executed inside the
service transaction which risks losing Redis sign-up tokens/passwords if the
transaction rolls back; move the clearSignUpData call out of the transactional
boundary by invoking it after commit using either
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) on an
application event (publish a SignUpCompletedEvent) or register a
TransactionSynchronization via
TransactionSynchronizationManager.registerSynchronization(...) to run
clearSignUpData in afterCommit; implement one of these approaches, remove the
direct call from the transactional method, and ensure tests cover rollback
scenarios to verify Redis data remains when the DB transaction fails.
There was a problem hiding this comment.
음.. 이렇게 하는게 좋을 것 같긴한데? 굳이? 싶기도 하네요..
코드도 복잡해질 것 같고, 이런 이벤트와 관련된 내용이 여러곳에서 사용된다면 모를까 여기에서만 있는건데... 🤔
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
음~ 저도 이정돈 무시해도 될 거 같아요!
clearSignUpData 가 맨 마지막에 실행되기도 하니 이후에 롤백될 일도 크게 없을 거 같아요!
nayonsoso
force-pushed
the
refactor/444-sign-up-token
branch
from
2ccb57f to
828d8a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (1)
67-73: 트랜잭션 롤백 시 Redis 데이터 유실 가능성. 커밋 후 정리로 변경 필요.
- 현재 clearSignUpData가 @transactional 범위 내에서 즉시 실행됩니다.
- DB 트랜잭션이 이후 단계에서 롤백되면, Redis의 토큰/비밀번호는 이미 삭제되어 재시도가 불가해집니다.
- afterCommit 훅으로 정리 로직을 이동하면, 2) 커밋 확정 후에만 정리가 수행되어 안전합니다.
적용 diff:
- // 회원가입을 위해 저장한 데이터(SignUpToken, 비밀번호) 삭제 - clearSignUpData(email, authType); + // 회원가입 데이터 정리는 트랜잭션 커밋 이후에 수행 + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + clearSignUpData(email, authType); + } + });추가 import (이 파일 상단에 추가 필요):
import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager;
- 이벤트 발행(@TransactionalEventListener) 방식도 대안이지만, 팀 의견처럼 단일 사용처라면 위 방식이 더 단순합니다.
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (2)
42-47: SignUpToken 파싱을 1회로 축소하면 일관성과 비용이 개선됩니다.
- 현재 validate + parseEmail + parseAuthType로 토큰 디코딩이 최대 3회 일어날 수 있습니다.
- validate 시 클레임을 반환하거나, 2) parse 메서드에서 email/authType을 함께 반환하는 DTO(예: SignUpTokenClaims) 도입을 제안합니다.
48-50: 임시 비밀번호 누락 시 에러코드 정밀화 제안.
- SIGN_UP_TOKEN_INVALID로 매핑하면 “토큰 불일치”와 “임시 비밀번호 없음” 케이스가 혼재됩니다.
- PASSWORD_TEMPORARY_NOT_FOUND(또는 유사 코드)로 구분하면, 2) 장애 분석과 사용자 피드백 메시지가 명확해집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/example/solidconnection/auth/service/PasswordTemporaryStorage.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/SignUpService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/PasswordTemporaryStorageTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/SignUpTokenProviderTest.java(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/solidconnection/auth/service/PasswordTemporaryStorage.java
- src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java
- src/test/java/com/example/solidconnection/auth/service/PasswordTemporaryStorageTest.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/com/example/solidconnection/auth/service/SignUpTokenProviderTest.java (2)
src/test/java/com/example/solidconnection/auth/service/SignInServiceTest.java (1)
DisplayName(18-69)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (1)
Nested(48-92)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (13)
src/test/java/com/example/solidconnection/auth/service/SignUpTokenProviderTest.java (11)
1-1: 패키지 구조 변경이 올바르게 반영되었습니다.OAuth 전용 패키지에서 일반적인 auth.service 패키지로 이동한 것이 SignUpTokenProvider의 통합 역할과 일치합니다.
28-29: 클래스명과 디스플레이 이름이 적절하게 변경되었습니다.
- 클래스명:
OAuthSignUpTokenProviderTest→SignUpTokenProviderTest로 변경- 디스플레이 이름: OAuth 전용에서 일반적인 "회원가입 토큰 제공자 테스트"로 변경
통합된 SignUpTokenProvider의 역할을 잘 반영하고 있습니다.
32-32: 의존성 주입 타입이 올바르게 변경되었습니다.
OAuthSignUpTokenProvider에서SignUpTokenProvider로 변경되어 통합된 토큰 제공자를 사용하도록 수정되었습니다.
43-46: 테스트 상수들이 적절하게 정의되었습니다.
authTypeClaimKey: 하드코딩된 문자열을 상수로 추출하여 유지보수성 향상authType: 테스트에서 반복 사용되는 값들을 상수로 정의하여 일관성 확보코드의 가독성과 유지보수성이 개선되었습니다.
50-50: 메서드 호출이 통합된 API에 맞게 변경되었습니다.
generateAndSaveSignUpToken메서드 호출 변경- 클레임 키를
authTypeClaimKey상수로 사용하도록 변경통합된 SignUpTokenProvider의 API를 올바르게 사용하고 있습니다.
Also applies to: 55-55
64-75: 새로운 토큰 삭제 테스트가 추가되었습니다.
deleteByEmail메서드의 기능을 검증하는 테스트 추가- Redis에서 토큰이 실제로 삭제되는지 확인
SignUpTokenProvider의 확장된 기능을 적절히 테스트하고 있습니다. 특히 Redis 키 삭제를 직접 확인하는 것은 좋은 접근입니다.
83-88: 검증 테스트가 통합된 API에 맞게 수정되었습니다.
- 클레임 생성 시
authTypeClaimKey상수 사용validateSignUpToken메서드 호출로 변경통합된 토큰 제공자의 검증 로직을 올바르게 테스트하고 있습니다.
97-97: 모든 예외 테스트가 통합된 API를 사용하도록 수정되었습니다.
- 만료된 토큰 검증
- JWT가 아닌 토큰 검증
- 잘못된 AuthType 검증
- Subject 누락 검증
- 서버 발급이 아닌 토큰 검증
모든 예외 시나리오에 대해
signUpTokenProvider.validateSignUpToken을 사용하여 일관성 있게 테스트하고 있습니다.Also applies to: 108-108, 121-121, 133-133, 145-145
116-118: AuthType 검증 테스트가 개선되었습니다.
- 잘못된 AuthType 값으로 "카카오" 문자열 사용
authTypeClaimKey상수를 사용하여 클레임 생성- Subject 없이 토큰 생성하여 형식 오류 테스트
AuthType enum 변환 실패 시나리오를 명확하게 테스트하고 있습니다.
154-155: 파싱 테스트들이 통합된 API에 맞게 수정되었습니다.
- 이메일 파싱:
parseEmail메서드 사용- AuthType 파싱:
parseAuthType메서드 사용- 모든 클레임에서
authTypeClaimKey상수 사용통합된 토큰 제공자의 파싱 기능을 올바르게 테스트하고 있습니다.
Also applies to: 159-159, 168-169, 172-172
180-180: 만료된 토큰 생성 메서드가 개선되었습니다.Subject를
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (2)
27-29: 핵심 흐름 통합 좋습니다. 응집도와 DIP 개선 확인.
- 서비스 단일화로 책임이 명확해졌습니다.
- 컨트롤러 단순화 및 테스트 용이성에도 도움이 됩니다.
87-93: 임시 비밀번호 저장소: TTL 및 해시 적용 확인 완료
- TTL 적용 검증
save메서드에서redisTemplate.opsForValue().set호출 시TokenType.SIGN_UP.getExpireTime()과TimeUnit.MILLISECONDS로 만료 시간이 설정되어 있습니다.- 해시 적용 검증
passwordEncoder.encode(rawPassword)호출로 저장 값이 단방향 해시(BCrypt 등)로 암호화되어 있습니다.따라서 추가 조치 없이 현재 구현을 유지하셔도 됩니다.
src/main/java/com/example/solidconnection/auth/service/SignUpService.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/service/SignUpService.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/service/SignUpService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (3)
68-71: 트랜잭션 내 clearSignUpData 삭제 호출 → 커밋 후 실행으로 이동하세요.트랜잭션이 롤백되면 Redis/임시 저장 데이터는 이미 삭제되어 재시도가 불가능해집니다. 커밋 이후에 정리되도록 등록하는 편이 안전합니다. (과거 동일 지적과 중복됨)
- 커밋 이후 콜백으로 정리하세요.
- 롤백 시에는 정리가 수행되지 않아 재시도가 가능해집니다.
- 테스트에 “DB 저장 실패 → 롤백 → 임시 데이터 보존” 시나리오를 추가하는 것을 권장합니다.
적용 diff:
- // 회원가입을 위해 저장한 데이터(SignUpToken, 비밀번호) 삭제 - clearSignUpData(email, authType); + // 커밋 후 회원가입 임시 데이터 정리 예약 + org.springframework.transaction.support.TransactionSynchronizationManager.registerSynchronization( + new org.springframework.transaction.support.TransactionSynchronization() { + @Override + public void afterCommit() { + clearSignUpData(email, authType); + } + } + );
51-59: 동시 가입 경쟁 조건 방지: save 시 UNIQUE 제약 예외를 도메인 에러로 매핑하세요.exists 체크만으로는 동시성에 취약합니다. DB UNIQUE 제약 위반을 잡아 동일한 도메인 에러로 매핑하면 안전합니다. (과거 동일 제안과 중복)
- save를 try-catch로 감싸고 DataIntegrityViolationException을 USER_ALREADY_EXISTED/NICKNAME_ALREADY_EXISTED로 매핑하세요.
- 메시지/제약명으로 구분이 어렵다면 보수적으로 USER_ALREADY_EXISTED로 매핑해도 됩니다.
- 단위테스트: 병렬 요청으로 레이스 발생 시 매핑 확인.
적용 diff:
- SiteUser siteUser = siteUserRepository.save(new SiteUser( - email, - signUpRequest.nickname(), - signUpRequest.profileImageUrl(), - signUpRequest.exchangeStatus(), - Role.MENTEE, - authType, - password - )); + SiteUser siteUser; + try { + siteUser = siteUserRepository.save(new SiteUser( + email, + signUpRequest.nickname(), + signUpRequest.profileImageUrl(), + signUpRequest.exchangeStatus(), + Role.MENTEE, + authType, + password + )); + } catch (org.springframework.dao.DataIntegrityViolationException e) { + throw mapToSignUpDuplicateException(e); + }추가 메서드(파일 내 임의 위치에 추가):
// UNIQUE 제약 위반 → 도메인 에러 매핑 private CustomException mapToSignUpDuplicateException(org.springframework.dao.DataIntegrityViolationException e) { String msg = (e.getMostSpecificCause() != null) ? e.getMostSpecificCause().getMessage() : ""; if (msg.contains("nickname")) { return new CustomException(NICKNAME_ALREADY_EXISTED); } return new CustomException(USER_ALREADY_EXISTED); }
94-99: 토큰 삭제 범위 좁히기: email 단독 → email+AuthType 기준으로 삭제하세요.동일 이메일로 다른 인증수단(AuthType) 가입 플로우가 공존하면 과잉 삭제 위험이 있습니다. (이전 코멘트와 동일 지적)
- Provider에 deleteByEmailAndAuthType(email, authType) 추가.
- 호출부를 해당 메서드로 교체.
- 테스트: 서로 다른 AuthType의 토큰이 상호 삭제되지 않음을 검증.
적용 diff:
- if (authType == AuthType.EMAIL) { - passwordTemporaryStorage.deleteByEmail(email); - } - signUpTokenProvider.deleteByEmail(email); + if (authType == AuthType.EMAIL) { + passwordTemporaryStorage.deleteByEmail(email); + } + signUpTokenProvider.deleteByEmailAndAuthType(email, authType);Provider 메서드 추가 예시(참고):
public interface SignUpTokenProvider { void deleteByEmailAndAuthType(String email, AuthType authType); // ... }
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (2)
62-63: 관심 지역/국가 입력 방어: null/중복 안전 처리로 NPE와 중복 저장을 예방하세요.DTO가 null을 허용하거나 중복을 포함하면 서비스 내부에서 NPE/중복 저장이 발생할 수 있습니다.
- null → empty 리스트로 대체.
- stream().distinct()로 중복 제거.
- 선택: 존재하지 않는 이름이 요청되면 조용히 누락되므로, 검증을 추가할지 정책을 결정.
적용 diff:
- interestedRegionService.saveInterestedRegion(siteUser, signUpRequest.interestedRegions()); - interestedCountryService.saveInterestedCountry(siteUser, signUpRequest.interestedCountries()); + interestedRegionService.saveInterestedRegion( + siteUser, + signUpRequest.interestedRegions() == null + ? java.util.Collections.emptyList() + : signUpRequest.interestedRegions().stream().distinct().toList() + ); + interestedCountryService.saveInterestedCountry( + siteUser, + signUpRequest.interestedCountries() == null + ? java.util.Collections.emptyList() + : signUpRequest.interestedCountries().stream().distinct().toList() + );
41-46: 토큰 처리 일관화: validate+parse 분리 대신 단일 디코드 반환을 고려해 가독성을 높이세요.성능 이슈는 아니지만, 한 번의 디코드로 이메일/AuthType을 받는 API가 있으면 호출부가 단순해집니다.
- SignUpTokenProvider.decode(token) → SignUpTokenClaims(email, authType) 반환.
- validate는 decode 내부에서 수행.
- 호출부는 claims.email()/claims.authType()만 사용.
예시:
var claims = signUpTokenProvider.decode(signUpRequest.signUpToken()); String email = claims.email(); AuthType authType = claims.authType();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (4)
src/main/java/com/example/solidconnection/siteuser/service/SiteUserService.java (1)
RequiredArgsConstructor(8-18)src/main/java/com/example/solidconnection/location/country/service/InterestedCountryService.java (1)
Service(12-27)src/main/java/com/example/solidconnection/location/region/service/InterestedRegionService.java (1)
Service(12-27)src/main/java/com/example/solidconnection/auth/service/oauth/OAuthService.java (1)
Service(24-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (1)
86-92: 1) TTL 적용 확인
PasswordTemporaryStorage.save()에서 redisTemplate.opsForValue().set(…, TokenType.SIGN_UP.getExpireTime(), TimeUnit.MILLISECONDS)로 명확히 만료시간을 설정하고 있어 TTL은 정상 동작 중입니다.2) 만료 시 에러 매핑 검토
만료된 키는 findByEmail에서 Optional.empty()를 반환하고, SignUpService.getTemporarySavedPassword()에서 SIGN_UP_TOKEN_INVALID로 처리됩니다.
• 현재 SIGN_UP_TOKEN_INVALID로 일관성 있게 처리되는 점은 무방하나,
• 만료 전용 에러코드(EXPIRED_SIGN_UP_PASSWORD 등)로 분리하고 싶다면 CustomException(ErrorCode.EXPIRED_TEMP_PASSWORD) 추가를 권장드립니다.3) 자동 정리 보장
• save() 시 TTL로 자동 삭제되므로 실패·롤백 후 별도 로직 없이도 만료 시 자동 정리됩니다.
• 추가로 clearSignUpData()를 통해 성공 흐름에서 즉시 삭제하므로 안전합니다.—
태그:
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
확인했습니다!! 빠르게 반영해주셔서 감사합니다 😊
3 participants
관련 이슈
작업 내용
특이 사항
커밋 따라가며 읽으시면 이해하기 쉽습니다~
주요 커밋
리뷰 요구사항 (선택)