Conversation
Walkthrough이 PR은 이메일 인증/비밀번호 재설정 응답 DTO를 분리·확장하고, 이메일 발송 흐름에서 소셜 제공자 연동 여부를 확인하도록 EmailService와 리포지토리 조회를 추가하며 컨트롤러 및 API 문서를 해당 DTO로 변경합니다. (≤50단어) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as UserController
participant Service as EmailService
participant Repo as AuthProviderAccountRepository
participant DB as Database
Client->>Controller: 이메일 인증/비밀번호 재설정 요청 (email, type)
Controller->>Service: 요청 전달 (email, type)
Service->>DB: User 존재 여부 확인 (email)
alt User 존재 (이미 계정 있음)
Service->>Repo: findByUserEmail(email)
Repo->>DB: AuthProviderAccount 조회
DB-->>Repo: provider 목록 반환
Repo-->>Service: provider 목록
Service->>Service: UserConverter.toEmailSentResponseFail(email, providers)
Service-->>Controller: EmailSentResponse(isProviderLinked=true, providerTypes=...)
else User 미존재 (새 요청)
Service->>Service: 인증코드 생성 및 메일 전송
Service->>Service: UserConverter.toEmailSentResponseSuccess(email) / toPasswordResetResponse(email)
Service-->>Controller: EmailSentResponse / PasswordResetResponse (isProviderLinked=false, expireIn=...)
end
Controller-->>Client: DataResponse<EmailSentResponse/PasswordResetResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
짧게 한 마디: 소셜 로그인으로 이미 존재하는 이메일에 대해 "가입 불가" 대신 제공자 목록을 반환하는 방향은 이슈 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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: 4
🧹 Nitpick comments (4)
src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java (1)
30-40: API 문서 업데이트 잘 되었습니다 👍, 한 가지 작은 제안이 있습니다.소셜 로그인 케이스에 대한 설명이 추가된 것이 좋습니다. 다만 Line 34에서
providerType = KAKAO로 특정 플랫폼만 예시하고 있는데, 이미 Google도 지원하고 있으므로providerType = KAKAO | GOOGLE 등처럼 복수 플랫폼을 명시하면 API 소비자가 더 정확하게 이해할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java` around lines 30 - 40, Update the API documentation string in UserControllerDocs for the sendEmail operation to indicate multiple social providers instead of only KAKAO; specifically, change the example fragment that currently reads "isProviderLinked = true, providerType = KAKAO" to include other supported providers (e.g., "providerType = KAKAO | GOOGLE" or "providerType = KAKAO, GOOGLE") so consumers of the sendEmail (EmailRequest.Send -> EmailSentResponse) API understand that multiple providerType values are possible.src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.java (1)
5-11: 하나의 Record가 두 가지 의미를 담고 있습니다 — 분리를 고려해보세요.현재
EmailSentResponse는 두 가지 서로 다른 상황을 표현합니다:
- ✅ 이메일 전송 성공:
expireIn=180L,isProviderLinked=false,providerType=null⚠️ 소셜 계정 연동됨:expireIn=null,isProviderLinked=true,providerType=KAKAOPR 설명에서 이 설계에 대한 논의가 있는 것으로 보이므로 현재는 괜찮지만, 향후 응답 구조가 더 복잡해지면 별도 Response 타입(예:
ProviderLinkedResponse)으로 분리하는 것이 SRP(단일 책임 원칙) 측면에서 더 깔끔합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.java` around lines 5 - 11, EmailSentResponse currently encodes two distinct cases (email-sent with expireIn and provider-linked with isProviderLinked/providerType); split into two focused response types to follow SRP: keep EmailSentResponse for the email flow (fields: message, email, expireIn) and introduce a separate ProviderLinkedResponse (fields: message, email, providerType) and update any code constructing or returning EmailSentResponse (factory/mapper methods, controllers, services) to return the appropriate type based on isProviderLinked instead of overloading EmailSentResponse.src/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.java (1)
39-60:expireIn = 180L매직 넘버가EmailService의 Redis TTL과 중복됩니다.
EmailService.emailSendTemplate(Line 139)에서 Redis TTL을60 * 3L(= 180초)로 설정하고 있고, 여기서도180L을 하드코딩하고 있습니다. 둘 중 하나만 변경되면 클라이언트에 보여주는 만료 시간과 실제 만료 시간이 달라지는 버그가 발생합니다.예를 들어 나중에 TTL을 5분(300초)으로 바꿨는데, 여기
180L을 깜빡하면 프론트엔드에서는 "3분 남았다"고 보여주지만 실제로는 5분간 유효한 상황이 됩니다.♻️ 상수 추출 제안
+// EmailService 또는 별도 상수 클래스에 정의 +public static final long EMAIL_CODE_EXPIRE_SECONDS = 180L;그리고
UserConverter와EmailService모두 이 상수를 참조하도록 변경합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.java` around lines 39 - 60, Extract a single shared constant for the email verification TTL (e.g., EMAIL_VERIFICATION_TTL_SECONDS) and use it everywhere instead of hardcoded 180L or 60 * 3L: update EmailService.emailSendTemplate to use the new constant for the Redis TTL and update UserConverter.toEmailSentResponseSuccess, toEmailSentResponseFail, and toPasswordResetResponse to return expireIn by referencing that same constant; place the constant in a shared config/constants class or a common util so both EmailService and UserConverter import it.src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java (1)
141-147: 문자열 기반 분기 대신enum을 고려해보세요.
type.equals("회원가입")과 같은 문자열 비교는 오타에 취약하고, 새로운 타입이 추가될 때 실수가 발생하기 쉽습니다. 예를 들어"회원가입"대신"회원 가입"(공백)을 전달하면 의도치 않게else분기로 빠지게 됩니다.public enum EmailType { SIGNUP("회원가입"), PASSWORD_RESET("비밀번호 재설정"); private final String label; // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java` around lines 141 - 147, Replace the fragile string comparison in EmailService (currently using type.equals("회원가입")) with a dedicated enum (e.g., EmailType { SIGNUP, PASSWORD_RESET }) and change the method signature or caller to accept EmailType instead of a raw string; then switch on EmailType (or use if EmailType.SIGNUP) and call UserConverter.toEmailSentResponseSuccess(toEmail) for SIGNUP and UserConverter.toPasswordResetResponse(toEmail) for PASSWORD_RESET. Ensure any parsing from incoming strings to EmailType uses valueOf/lookup with clear error handling to avoid IllegalArgumentException.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java`:
- Around line 44-63: Replace the redundant existsByEmail + findUserByEmail
pattern in sendEmail by calling userRepository.findUserByEmail once and using
the returned Optional to branch; i.e., remove the existsByEmail call, call
findUserByEmail(toEmail) into an Optional<User>, if empty proceed with the "no
user" path (or throw USER_NOT_FOUND where appropriate), and if present retrieve
the user, query authProviderAccountRepository.findByUser(user) and preserve the
existing logic that throws USER_EMAIL_DUPLICATE when authProviderAccount is
empty or returns the social provider via UserConverter.toEmailSentResponseFail
when present.
- Around line 98-100: The method emailSendTemplate currently returns Object
which breaks type safety and forces unsafe casts; change
emailSendTemplate(String toEmail, String type) to perform only the common
side-effects (generate code, save to Redis, send email) and return void (or a
dedicated sealed/result type if you prefer), then update callers (sendEmail and
sendEmailForPwd) to call emailSendTemplate(...) and convert to their specific
DTOs via UserConverter.toEmailSentResponseSuccess(toEmail) and
UserConverter.toPasswordResetResponse(toEmail) respectively; ensure any errors
are propagated as exceptions handled by the global exception handler and remove
all Object casts.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.java`:
- Around line 14-15: The repository method findByUser in
AuthProviderAccountRepository can throw NonUniqueResultException if a User has
multiple AuthProviderAccount rows; change the method signature to return
List<AuthProviderAccount> instead of Optional<AuthProviderAccount> (or
alternatively provide findFirstByUser if only the first is desired) and update
any callers (e.g., EmailService) to handle a list (choose first element or
iterate as appropriate). Ensure the `@Query` or method name is consistent with the
new return type and adjust EmailService usages to access the chosen
AuthProviderAccount(s).
- Line 5: The file AuthProviderAccountRepository incorrectly imports
io.lettuce.core.dynamic.annotation.Param (Redis) causing Spring Data parameter
binding failures; replace that import with
org.springframework.data.repository.query.Param and ensure the repository's
`@Query` (or method parameter) uses that `@Param`. Also address the potential
multi-account bug in findByUser: either change the method to return
List<AuthProviderAccount> (or Optional<List<AuthProviderAccount>>) or narrow the
query by adding a provider filter (e.g., where apa.user = :user AND apa.provider
= :provider) depending on whether a User may have multiple AuthProviderAccount
entries.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.java`:
- Around line 5-11: EmailSentResponse currently encodes two distinct cases
(email-sent with expireIn and provider-linked with
isProviderLinked/providerType); split into two focused response types to follow
SRP: keep EmailSentResponse for the email flow (fields: message, email,
expireIn) and introduce a separate ProviderLinkedResponse (fields: message,
email, providerType) and update any code constructing or returning
EmailSentResponse (factory/mapper methods, controllers, services) to return the
appropriate type based on isProviderLinked instead of overloading
EmailSentResponse.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.java`:
- Around line 39-60: Extract a single shared constant for the email verification
TTL (e.g., EMAIL_VERIFICATION_TTL_SECONDS) and use it everywhere instead of
hardcoded 180L or 60 * 3L: update EmailService.emailSendTemplate to use the new
constant for the Redis TTL and update UserConverter.toEmailSentResponseSuccess,
toEmailSentResponseFail, and toPasswordResetResponse to return expireIn by
referencing that same constant; place the constant in a shared config/constants
class or a common util so both EmailService and UserConverter import it.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java`:
- Around line 141-147: Replace the fragile string comparison in EmailService
(currently using type.equals("회원가입")) with a dedicated enum (e.g., EmailType {
SIGNUP, PASSWORD_RESET }) and change the method signature or caller to accept
EmailType instead of a raw string; then switch on EmailType (or use if
EmailType.SIGNUP) and call UserConverter.toEmailSentResponseSuccess(toEmail) for
SIGNUP and UserConverter.toPasswordResetResponse(toEmail) for PASSWORD_RESET.
Ensure any parsing from incoming strings to EmailType uses valueOf/lookup with
clear error handling to avoid IllegalArgumentException.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java`:
- Around line 30-40: Update the API documentation string in UserControllerDocs
for the sendEmail operation to indicate multiple social providers instead of
only KAKAO; specifically, change the example fragment that currently reads
"isProviderLinked = true, providerType = KAKAO" to include other supported
providers (e.g., "providerType = KAKAO | GOOGLE" or "providerType = KAKAO,
GOOGLE") so consumers of the sendEmail (EmailRequest.Send -> EmailSentResponse)
API understand that multiple providerType values are possible.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/PasswordResetResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/UserController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java
Comment on lines
44
to
63
| public EmailSentResponse sendEmail(String toEmail) { | ||
| if (userRepository.existsByEmail(toEmail)) { // 이미 해당 이메일로 생성한 계정이 있으면 | ||
| throw new UserHandler(UserErrorCode.USER_EMAIL_DUPLICATE); // 이메일 중복 예외(회원가입 시 사용했던 예외) | ||
| //해당 사용자 정보 조회 | ||
| User user = userRepository.findUserByEmail(toEmail) | ||
| .orElseThrow(() -> new UserHandler(UserErrorCode.USER_NOT_FOUND)); | ||
|
|
||
| //해당 사용자에 연관된 AuthProviderAccount 조회 | ||
| Optional<AuthProviderAccount> authProviderAccount = authProviderAccountRepository.findByUser(user); | ||
|
|
||
| //만약 AuthProviderAccount 가 없으면 | ||
| if (authProviderAccount.isEmpty()) { | ||
| //단순 이메일 회원가입에서 이메일 값이 중복인 것이므로 예외(기존 예외처리 로직) | ||
| throw new UserHandler(UserErrorCode.USER_EMAIL_DUPLICATE); | ||
| } else { //만약 AuthProviderAccount 가 있으면 | ||
| //해당 소셜 로그인 플랫폼 타입을 추출해서 반환 | ||
| Provider provider = authProviderAccount.get().getProvider(); | ||
|
|
||
| return UserConverter.toEmailSentResponseFail(toEmail, provider); | ||
| } | ||
| } |
There was a problem hiding this comment.
불필요한 DB 쿼리 중복: existsByEmail + findUserByEmail
Line 45에서 existsByEmail로 존재 여부를 확인한 후, Line 47에서 다시 findUserByEmail로 같은 이메일을 조회합니다. 이는 동일 데이터에 대해 DB를 두 번 조회하는 것입니다.
findUserByEmail만 사용하여 Optional의 isPresent()로 판단하면 쿼리 1회로 줄일 수 있습니다.
♻️ 쿼리 최적화 제안
public EmailSentResponse sendEmail(String toEmail) {
- if (userRepository.existsByEmail(toEmail)) {
- User user = userRepository.findUserByEmail(toEmail)
- .orElseThrow(() -> new UserHandler(UserErrorCode.USER_NOT_FOUND));
+ Optional<User> existingUser = userRepository.findUserByEmail(toEmail);
+ if (existingUser.isPresent()) {
+ User user = existingUser.get();
Optional<AuthProviderAccount> authProviderAccount = authProviderAccountRepository.findByUser(user);As per coding guidelines, "JPA 사용 시 N+1 문제나 불필요한 쿼리가 발생하지 않는지 ... 체크하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java`
around lines 44 - 63, Replace the redundant existsByEmail + findUserByEmail
pattern in sendEmail by calling userRepository.findUserByEmail once and using
the returned Optional to branch; i.e., remove the existsByEmail call, call
findUserByEmail(toEmail) into an Optional<User>, if empty proceed with the "no
user" path (or throw USER_NOT_FOUND where appropriate), and if present retrieve
the user, query authProviderAccountRepository.findByUser(user) and preserve the
existing logic that throws USER_EMAIL_DUPLICATE when authProviderAccount is
empty or returns the social provider via UserConverter.toEmailSentResponseFail
when present.
src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java
Show resolved
Hide resolved
...whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.java
Outdated
Show resolved
Hide resolved
...whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.java
Outdated
Show resolved
Hide resolved
ojy0903
changed the title
kingmingyu
approved these changes
Feb 23, 2026
Collaborator
kingmingyu
left a comment
kingmingyu
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다! 저도 400번대 에러를 반환하는 것 보다는 성공 응답에 안내 문구를 넣는 것이 괜찮아보입니다! 추가로 아까 회의 때 테스트가 안된다는 pr이 마이페이지쪽 맞을까요??
.../java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.java
Outdated
Show resolved
Hide resolved
Collaborator
Author
네 맞습니다! 소셜로그인에서 예시로 네이버 소셜 로그인 회원가입에 쓰는 이메일(ojy654@naver.com) 과, 카카오 소셜 로그인 회원가입에 쓰는 이메일(ojy654@naver.com) 이 동일할 경우에 네이버로 로그인하고 마이페이지 API 호출시 정상적으로 provider 가 NAVER 로 뜨는지 와 같이 "같은 이메일 값으로 각각 다른 플랫폼으로 로그인 시 마이페이지에서 provider 가 정상적으로 나오는지" 를 테스트 못했었습니다ㅠㅠ |
Collaborator
확인했습니다!! 내일 테스트 해보겠습니다!! 추가로 저희 배포 서버 swagger가 뭔가 최신화가 안되고 있는 것 같은데 저만 그런가요?? |
# Conflicts: # src/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.java # src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/UserController.java # src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java (1)
98-147: 🛠️ Refactor suggestion | 🟠 Major
Object반환 타입은 타입 안전성을 해칩니다 — 이전 리뷰에서 지적된 사항
emailSendTemplate이Object를 반환하고 Line 68, 75에서 강제 캐스팅하는 구조는 여전히ClassCastException위험이 있습니다. 이전 리뷰에서 제안된 대로, 템플릿 메서드는 공통 로직(코드 생성, Redis 저장, 이메일 전송)만 담당하고void를 반환하며, DTO 변환은 호출부에서 처리하는 것이 SOLID 원칙에 더 부합합니다.♻️ void 반환 + 호출부에서 DTO 생성
- private Object emailSendTemplate(String toEmail, String type) { + private void emailSendTemplate(String toEmail, String type) { // ... 기존 코드 생성, 이메일 전송, Redis 저장 로직 동일 ... redisUtil.setDataExpire("CODE:" + toEmail, authCode, 60 * 3L); - if (type.equals("회원가입")) { - return UserConverter.toEmailSentResponseSuccess(toEmail); - } else { - return UserConverter.toPasswordResetResponse(toEmail); - } }호출부:
// sendEmail: emailSendTemplate(toEmail, "회원가입"); return UserConverter.toEmailSentResponseSuccess(toEmail); // sendEmailForPwd: emailSendTemplate(toEmail, "비밀번호 재설정"); return UserConverter.toPasswordResetResponse(toEmail);이렇게 하면
Object캐스팅이 완전히 제거되고, 새로운 이메일 유형이 추가되더라도 컴파일 타임에 타입 안전성이 보장됩니다.As per coding guidelines, "SOLID 원칙, 의존성 주입(DI), 예외 처리(GlobalExceptionHandler)가 적절한지 보라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java` around lines 98 - 147, emailSendTemplate currently returns Object causing unsafe casts; change emailSendTemplate(String toEmail, String type) to return void and limit it to shared behavior (createCode, Redis ops, isTestEmail logging, actual send and MailException handling), remove the conditional returns inside emailSendTemplate, and keep only redisUtil.setDataExpire and email sending logic there; then update the callers (sendEmail and sendEmailForPwd) to call emailSendTemplate(toEmail, "회원가입") or emailSendTemplate(toEmail, "비밀번호 재설정") and return the appropriate DTOs via UserConverter.toEmailSentResponseSuccess(toEmail) and UserConverter.toPasswordResetResponse(toEmail) respectively so type safety is restored and Object casts are eliminated.
🧹 Nitpick comments (4)
src/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.java (1)
54-75: 매직 넘버180L이 중복되어 있습니다 — 상수로 추출 권장
toEmailSentResponseSuccess(Line 57)와toPasswordResetResponse(Line 74)에서180L이 반복되고,EmailService의 Redis TTL(60 * 3L = 180초, Line 139)과도 동일한 값입니다.나중에 Redis TTL만 변경하고 이 값들을 깜빡하면, 클라이언트에게 잘못된 만료 시간을 안내하게 됩니다. 하나의 상수로 추출해두면 이런 실수를 방지할 수 있어요.
♻️ 상수 추출 예시
EmailService또는 별도 상수 클래스에서:public static final long VERIFICATION_CODE_EXPIRE_SECONDS = 180L;그리고
UserConverter와EmailService에서 이 상수를 참조:- 180L, + EmailService.VERIFICATION_CODE_EXPIRE_SECONDS,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.java` around lines 54 - 75, Extract the duplicated literal 180L into a named constant (e.g., VERIFICATION_CODE_EXPIRE_SECONDS) in a shared location (EmailService or a new constants class) and replace the hardcoded values in UserConverter.toEmailSentResponseSuccess and UserConverter.toPasswordResetResponse with that constant; also update EmailService (where TTL is computed as 60 * 3L) to reference the same constant so all TTL/expiry messaging is driven from a single source of truth.src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java (2)
44-62:existsByEmail+findByUserEmail쿼리 최적화 가능Line 45에서
userRepository.existsByEmail(toEmail)로 사용자 존재 여부를 확인하고, Line 47에서authProviderAccountRepository.findByUserEmail(toEmail)로 소셜 계정을 조회합니다. 이 두 쿼리는 서로 다른 테이블을 대상으로 하지만, 순서를 바꾸면 소셜 계정이 있는 경우 쿼리 1회로 줄일 수 있습니다:
- 먼저
findByUserEmail(toEmail)호출- 결과가 비어있지 않으면 → 소셜 계정 존재 (바로 반환)
- 비어있으면 →
existsByEmail(toEmail)로 일반 계정 중복 체크소셜 계정이 있는 경우가 이 PR의 핵심 시나리오이므로, 해당 경로에서 DB 호출이 1회로 줄어듭니다.
♻️ 쿼리 최적화 제안
public EmailSentResponse sendEmail(String toEmail) { - if (userRepository.existsByEmail(toEmail)) { - List<AuthProviderAccount> authProviderAccounts = authProviderAccountRepository.findByUserEmail(toEmail); - - if (authProviderAccounts.isEmpty()) { - throw new UserHandler(UserErrorCode.USER_EMAIL_DUPLICATE); - } else { - List<Provider> providers = new ArrayList<>(); - for (AuthProviderAccount authProviderAccount : authProviderAccounts) { - Provider provider = authProviderAccount.getProvider(); - providers.add(provider); - } - - return UserConverter.toEmailSentResponseFail(toEmail, providers); - } + List<AuthProviderAccount> authProviderAccounts = authProviderAccountRepository.findByUserEmail(toEmail); + + if (!authProviderAccounts.isEmpty()) { + List<Provider> providers = authProviderAccounts.stream() + .map(AuthProviderAccount::getProvider) + .toList(); + return UserConverter.toEmailSentResponseFail(toEmail, providers); + } + + if (userRepository.existsByEmail(toEmail)) { + throw new UserHandler(UserErrorCode.USER_EMAIL_DUPLICATE); }As per coding guidelines, "JPA 사용 시 N+1 문제나 불필요한 쿼리가 발생하지 않는지 ... 체크하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java` around lines 44 - 62, The current sendEmail method first calls userRepository.existsByEmail(toEmail) then authProviderAccountRepository.findByUserEmail(toEmail), causing an extra DB call; refactor sendEmail to call authProviderAccountRepository.findByUserEmail(toEmail) first, if the returned List<AuthProviderAccount> is not empty build providers and return UserConverter.toEmailSentResponseFail(toEmail, providers), otherwise fall back to calling userRepository.existsByEmail(toEmail) and throw UserHandler(UserErrorCode.USER_EMAIL_DUPLICATE) when appropriate—this reduces one query for the common social-account path.
55-59: Java 17 Stream API 활용 제안
for루프 대신stream()을 사용하면 더 간결하고 가독성이 좋아집니다.♻️ 스트림 리팩토링
- List<Provider> providers = new ArrayList<>(); - for (AuthProviderAccount authProviderAccount : authProviderAccounts) { - Provider provider = authProviderAccount.getProvider(); - providers.add(provider); - } + List<Provider> providers = authProviderAccounts.stream() + .map(AuthProviderAccount::getProvider) + .toList();
toList()는 Java 16+에서 사용 가능하며, 불변 리스트를 반환합니다. 이 프로젝트가 Java 17을 사용하고 있으니 적합합니다.As per coding guidelines, "Java 17의 최신 문법(Record, Switch Expression, Text Block 등)을 적절히 사용하는지 확인하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java` around lines 55 - 59, Replace the explicit for-loop that builds providers from authProviderAccounts with a Stream API call: in EmailService (the block that declares List<Provider> providers and iterates authProviderAccounts using AuthProviderAccount.getProvider()), use authProviderAccounts.stream().map(AuthProviderAccount::getProvider).toList() to produce providers in a concise, idiomatic Java 17 style.src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.java (1)
7-13:providerTypes가null일 때 NPE 가능성 — 빈 리스트 기본값 고려
UserConverter.toEmailSentResponseSuccess에서providerTypes를null로 설정하고 있는데, 이 응답을 소비하는 쪽(프론트엔드 또는 내부 로직)에서providerTypes를null체크 없이 반복하면NullPointerException이 발생할 수 있습니다.예를 들어 프론트에서
response.providerTypes.forEach(...)같이 호출하면 터지겠죠?null대신Collections.emptyList()를 사용하면 방어적으로 더 안전합니다.다만 이 값은 JSON 직렬화 시
null로 나가므로, 프론트엔드에서isProviderLinked플래그로 분기하고 있다면 당장 문제되지 않을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.java` around lines 7 - 13, The EmailSentResponse.record's providerTypes can be null and cause NPEs; update usage in UserConverter.toEmailSentResponseSuccess (or the EmailSentResponse construction) to ensure providerTypes is never null by supplying an immutable empty list (e.g., Collections.emptyList() or List.of()) when the source value is null, or wrap the incoming list with Optional.ofNullable(...).orElse(Collections.emptyList()) so EmailSentResponse.providerTypes is always a safe, non-null list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java`:
- Around line 36-37: The Swagger description references a non-existent field
name "providerType = KAKAO"; update the docs to match the actual
EmailSentResponse DTO by using the correct field name "providerTypes"
(List<Provider>) and show an example value (e.g., providerTypes = [KAKAO]) and
keep the mention of isProviderLinked; locate the text near UserControllerDocs
and replace the incorrect "providerType = KAKAO" snippet with "providerTypes =
[KAKAO]" so it reflects the DTO shape and avoids front-end confusion.
---
Duplicate comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java`:
- Around line 98-147: emailSendTemplate currently returns Object causing unsafe
casts; change emailSendTemplate(String toEmail, String type) to return void and
limit it to shared behavior (createCode, Redis ops, isTestEmail logging, actual
send and MailException handling), remove the conditional returns inside
emailSendTemplate, and keep only redisUtil.setDataExpire and email sending logic
there; then update the callers (sendEmail and sendEmailForPwd) to call
emailSendTemplate(toEmail, "회원가입") or emailSendTemplate(toEmail, "비밀번호 재설정") and
return the appropriate DTOs via
UserConverter.toEmailSentResponseSuccess(toEmail) and
UserConverter.toPasswordResetResponse(toEmail) respectively so type safety is
restored and Object casts are eliminated.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.java`:
- Around line 7-13: The EmailSentResponse.record's providerTypes can be null and
cause NPEs; update usage in UserConverter.toEmailSentResponseSuccess (or the
EmailSentResponse construction) to ensure providerTypes is never null by
supplying an immutable empty list (e.g., Collections.emptyList() or List.of())
when the source value is null, or wrap the incoming list with
Optional.ofNullable(...).orElse(Collections.emptyList()) so
EmailSentResponse.providerTypes is always a safe, non-null list.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.java`:
- Around line 54-75: Extract the duplicated literal 180L into a named constant
(e.g., VERIFICATION_CODE_EXPIRE_SECONDS) in a shared location (EmailService or a
new constants class) and replace the hardcoded values in
UserConverter.toEmailSentResponseSuccess and
UserConverter.toPasswordResetResponse with that constant; also update
EmailService (where TTL is computed as 60 * 3L) to reference the same constant
so all TTL/expiry messaging is driven from a single source of truth.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.java`:
- Around line 44-62: The current sendEmail method first calls
userRepository.existsByEmail(toEmail) then
authProviderAccountRepository.findByUserEmail(toEmail), causing an extra DB
call; refactor sendEmail to call
authProviderAccountRepository.findByUserEmail(toEmail) first, if the returned
List<AuthProviderAccount> is not empty build providers and return
UserConverter.toEmailSentResponseFail(toEmail, providers), otherwise fall back
to calling userRepository.existsByEmail(toEmail) and throw
UserHandler(UserErrorCode.USER_EMAIL_DUPLICATE) when appropriate—this reduces
one query for the common social-account path.
- Around line 55-59: Replace the explicit for-loop that builds providers from
authProviderAccounts with a Stream API call: in EmailService (the block that
declares List<Provider> providers and iterates authProviderAccounts using
AuthProviderAccount.getProvider()), use
authProviderAccounts.stream().map(AuthProviderAccount::getProvider).toList() to
produce providers in a concise, idiomatic Java 17 style.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/whereyouad/WhereYouAd/domains/user/application/dto/response/EmailSentResponse.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/application/mapper/UserConverter.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/EmailService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/UserController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java
src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java (1)
36-37:⚠️ Potential issue | 🟡 Minor
providerType = [KAKAO, NAVER...]표기가 실제 응답 형식과 불일치합니다.PR 이미지에서 확인된 실제 API 응답은 아래와 같이 단일 문자열입니다:
"providerType": "KAKAO"하지만 현재 설명의
providerType = [KAKAO, NAVER...]는 배열(리스트) 형식처럼 보입니다. 프론트엔드 개발자 입장에서 이 표기만 보면providerType이 배열인지 단일 값인지 혼동할 수 있습니다.
EmailSentResponseDTO의providerType필드 타입이 단일 문자열/Enum이라면, 아래처럼 수정하는 것이 더 명확합니다:📝 문서 수정 제안
- "이미 소셜 로그인으로 가입된 이메일 값이 요청으로 들어올 경우, isProviderLinked = true, providerType = [KAKAO, NAVER...] 와 같이 값이 나오며 이메일은 전송되지 않습니다." + "이미 소셜 로그인으로 가입된 이메일 값이 요청으로 들어올 경우, isProviderLinked = true, providerType = KAKAO (KAKAO, NAVER, GOOGLE 중 하나) 와 같이 값이 나오며 이메일은 전송되지 않습니다."아울러,
EmailSentResponseDTO의 실제providerType필드 타입을 확인해주세요.#!/bin/bash # EmailSentResponse DTO의 providerType 필드 타입 확인 fd "EmailSentResponse.java" --exec cat {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java` around lines 36 - 37, The documentation text in UserControllerDocs misrepresents providerType as an array ("providerType = [KAKAO, NAVER...]") while the API returns a single string/enum; update the description in UserControllerDocs to show providerType as a single value (e.g., providerType: "KAKAO") and clarifying text, and verify the EmailSentResponse DTO's providerType field (EmailSentResponse.providerType) is indeed a single string/enum and adjust the docs to match that type if different.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java`:
- Around line 36-37: The documentation text in UserControllerDocs misrepresents
providerType as an array ("providerType = [KAKAO, NAVER...]") while the API
returns a single string/enum; update the description in UserControllerDocs to
show providerType as a single value (e.g., providerType: "KAKAO") and clarifying
text, and verify the EmailSentResponse DTO's providerType field
(EmailSentResponse.providerType) is indeed a single string/enum and adjust the
docs to match that type if different.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java
jinnieusLab
approved these changes
Feb 26, 2026
Collaborator
jinnieusLab
left a comment
jinnieusLab
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다! 응답 쪽에서는 말씀해주신 대로 200 OK 응답으로 하되 프론트 측에서 isProviderLinked 필드 값에 따라 처리하도록 하면 될 것 같습니다!!
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
3 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.
📌 관련 이슈
🚀 개요
기존에 소셜로그인으로 회원가입된 이메일로 기본 이메일 회원가입을 위한 인증코드 전송 요청 시,
소셜 계정이 연동되어 있음을 알리도록 수정
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit
새로운 기능
문서