[SRLT-116] Member, Auth 도메인 패키지 구조를 정리한다 #69
Conversation
- AuthenticatedMember 도입으로 어댑터 간 결합 완화 - Auth reissue 인자 memberId로 단순화 - MemberQueryUseCase/Service 네이밍 및 테스트 리팩토링 - Member 컨트롤러 조회 로직 정리
|
Note
|
| Cohort / File(s) | 설명 |
|---|---|
인증 도메인 이동·신규 타입 src/main/java/starlight/shared/auth/AuthenticatedMember.java, src/main/java/starlight/application/member/auth/..., src/main/java/starlight/adapter/member/auth/... |
인증 관련 패키지 이동(starlight.adapter.auth.* → starlight.adapter.member.auth.*), AuthenticatedMember 추가, AuthUseCase/AuthServiceImpl/AuthTokenResult/SignInInput/SignUpInput 등 신규·재정의 |
컨트롤러·Swagger 시그니처 변경 src/main/java/starlight/adapter/*/webapi/*Controller.java, src/main/java/starlight/adapter/*/webapi/swagger/*ApiDoc.java |
@AuthenticationPrincipal 파라미터 타입 변경(AuthDetails → AuthenticatedMember 또는 이동된 AuthDetails), AuthController 삭제/신규 추가, Swagger 인터페이스 갱신 |
토큰 추출기·토큰 프로바이더 리팩터링 src/main/java/starlight/adapter/member/auth/webapi/AuthTokenResolver.java, src/main/java/starlight/adapter/member/auth/security/jwt/JwtTokenProvider.java, src/main/java/starlight/application/member/auth/required/TokenProvider.java |
헤더 토큰 추출 책임을 분리(AuthTokenResolver), TokenProvider 메서드명(issue/reissue/logout 등)·반환 DTO(AuthTokenResult) 변경, prefix 처리 및 deprecated 주석 추가 |
JWT 핸들러·필터 재작성 src/main/java/starlight/adapter/member/auth/security/handler/*, src/main/java/starlight/adapter/member/auth/security/filter/* |
AccessDenied/AuthenticationEntryPoint 재작성(응답 JSON 직렬화), JwtFilter에 Redis KeyValueMap·AuthTokenResolver 도입 및 검증 로직 보강 |
포트·퍼시스턴스 명명 통일 src/main/java/starlight/application/*/required/*, src/main/java/starlight/adapter/*/persistence/* |
getOrThrow → findByIdOrThrow 등 명명 통일, Repository → Query/Command Port 분리(MemberQueryPort/MemberCommandPort), MemberJpa 등 포트 구현 변경 |
DTO·레코드 네이밍/형식 변경 src/main/java/starlight/application/*/provided/dto/*, src/main/java/starlight/adapter/*/webapi/dto/* |
*Dto → *Result, FeedbackRequestDto → FeedbackRequestInput, 웹/앱 레이어의 Input/Result 규칙 적용 및 변환 메서드 업데이트 |
결제 클라이언트 포트화 src/main/java/starlight/adapter/order/toss/TossClient.java, src/main/java/starlight/application/order/required/PaymentGatewayPort.java, src/main/java/starlight/application/order/provided/dto/TossClientResult.java |
TossClient가 PaymentGatewayPort 구현으로 변경, TossClientResponse → TossClientResult로 DTO명 변경, confirm/cancel 시그니처와 검증/예외 처리 조정 |
expert-report 관련 리네이밍 및 매핑 변경 src/main/java/starlight/adapter/expertReport/persistence/*, .../webapi/*, src/main/java/starlight/application/expertReport/provided/dto/ExpertReportWithExpertResult.java |
Repository/JPA/서비스/컨트롤러에서 메서드명(예: findByTokenWithCommentsOrThrow), DTO(ExpertReportWithExpertDto → ExpertReportWithExpertResult) 및 매핑(applicationCount 포함) 반영 |
이벤트·이메일 DTO 연계 변경 src/main/java/starlight/application/expertApplication/event/*, src/main/java/starlight/adapter/expertApplication/email/SMTPEmailSender.java |
FeedbackRequestDto → FeedbackRequestInput 이름 변경 및 관련 시그니처·이벤트/리스너/이메일 전송자 반영 |
테스트·문서·가이드 업데이트 src/test/..., 개발가이드.md |
패키지·인터페이스·메서드명·DTO 변경을 테스트 전반에 반영, 개발가이드에 포트·네이밍 규칙 문서화 |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant AuthController
participant AuthUseCase
participant TokenProvider
participant Redis
rect rgba(0,128,128,0.06)
Note over Client,AuthController: Sign-in 흐름 (리팩터링)
Client->>AuthController: POST /v1/auth/sign-in (SignInInput)
AuthController->>AuthUseCase: signIn(SignInInput)
AuthUseCase->>TokenProvider: issueTokens(member)
TokenProvider-->>AuthUseCase: AuthTokenResult(access, refresh)
AuthUseCase->>Redis: setValue(refreshToken, memberId, expiry)
Redis-->>AuthUseCase: OK
AuthUseCase-->>AuthController: AuthTokenResult
AuthController-->>Client: 200 ApiResponse(TokenResponse.from(...))
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- FEAT: 일반 로그인 및 소셜 로그인을 구현한다 #12 — 인증 시스템(토큰, OAuth2, AuthDetails, Jwt 필터/핸들러) 대규모 리팩터링과 직접적인 중복 변경이 다수 존재합니다.
- [SRLT-25] 토큰 기반 링크 유효성 설계 #36 — expert-report 관련 DTO·쿼리·컨트롤러 변경과 코드 레벨 중복이 있습니다.
- [SRLT-33] 전문가를 고르고 피드백을 요청한다 #26 — expert-application 이메일/피드백 요청 흐름 및 DTO 변경과 연관됩니다.
Suggested labels
🧵 REFACTOR, :memo: DOCS
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 변경 내용의 핵심을 명확하게 요약합니다. 'Member, Auth 도메인 패키지 구조를 정리한다'는 광범위한 리팩토링의 주요 목표를 간결하게 표현하며, 패키지 구조 정리라는 핵심 의도를 효과적으로 전달합니다. |
✨ Finishing touches
- 📝 Generate docstrings
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/starlight/adapter/member/auth/security/oauth2/OAuth2Attributes.java (2)
31-38: 프로바이더 ID 검증 로직에 잠재적 이슈가 있습니다.Line 31의
String.valueOf(response.getOrDefault("id", ""))는id값이 명시적으로null인 경우"null"문자열을 반환합니다. 이는isBlank()검증을 우회하게 됩니다.더 안전한 접근법은 다음과 같습니다:
🔎 권장 수정안
-String id = String.valueOf(response.getOrDefault("id", "")); +Object idObj = response.get("id"); +String id = (idObj != null) ? idObj.toString() : "";또는 null 체크를 명시적으로 추가:
String id = String.valueOf(response.getOrDefault("id", "")); -if (id.isBlank()) { +if (id == null || id.isBlank() || "null".equals(id)) { throw new OAuth2AuthenticationException(new OAuth2Error("invalid_provider_id"), "Missing provider id"); }
45-52: Kakao 프로바이더에도 동일한 검증 이슈가 있습니다.Line 45에서 네이버와 동일한
String.valueOf()패턴을 사용하므로, 명시적null값에 대한 처리가 필요합니다.🔎 권장 수정안
-String id = String.valueOf(attributes.getOrDefault("id", "")); +Object idObj = attributes.get("id"); +String id = (idObj != null) ? idObj.toString() : "";
🧹 Nitpick comments (17)
src/main/java/starlight/adapter/aireport/webapi/swagger/ImageApiDoc.java (1)
14-14: 인증 추상화 인터페이스로의 전환이 잘 적용되었습니다.API 문서가 구체 타입(
AuthDetails)에서 인터페이스(AuthenticatedMember)로 변경되어 더 나은 추상화를 제공합니다. 이는 의존성 역전 원칙(DIP)을 잘 따르는 설계입니다.🔎 변수명 일관성 개선 제안
Line 49의 파라미터 변수명이
authDetails로 되어 있는데, 타입이AuthenticatedMember로 변경되었으므로 변수명도 타입과 일관되게 변경하는 것을 고려해보세요:ApiResponse<PreSignedUrlResponse> getPresignedUrl( - @AuthenticationPrincipal AuthenticatedMember authDetails, + @AuthenticationPrincipal AuthenticatedMember authenticatedMember, @io.swagger.v3.oas.annotations.Parameter(description = "파일명", required = true) @RequestParam String fileName );Also applies to: 49-49
src/main/java/starlight/application/member/auth/provided/dto/AuthMemberResult.java (1)
1-20: 인증 결과 DTO 설계가 적절합니다.
AuthMemberResult레코드가 도메인 엔티티(Member)와 애플리케이션 계층을 명확히 분리하고 있습니다.application.member.auth.provided.dto패키지 위치도 포트 기반 아키텍처에 부합하며, 정적 팩토리 메서드를 통한 변환 패턴도 깔끔합니다.선택적 개선 사항:
phoneNumber필드가 PII(개인식별정보)에 해당하므로, 인증 컨텍스트에서 실제로 필요한지 검토해보시기 바랍니다. 불필요하게 노출되는 개인정보를 최소화하는 것이 GDPR/CCPA 관점에서 바람직합니다.src/main/java/starlight/application/member/provided/MemberQueryUseCase.java (2)
6-6: Use Case 네이밍이 개선되었지만 역할 분리를 고려해보세요.
MemberService를MemberQueryUseCase로 변경하여 유스케이스 인터페이스임을 명확히 했습니다. 이는 아키텍처 의도를 잘 드러냅니다.다만
QueryUseCase에createUser같은 커맨드 오퍼레이션이 포함되어 있어 CQRS 관점에서 일관성이 떨어집니다. 다음과 같은 분리를 고려해보시기 바랍니다:
MemberQueryUseCase:getUserByEmail,getUserById(조회 전용)MemberCommandUseCase:createUser(변경 작업)이미 포트 계층에서
MemberCommandPort와MemberQueryPort가 분리되어 있다면, 유스케이스 레벨에서도 동일한 분리 원칙을 적용하는 것이 일관성 있는 설계입니다.
8-8: 파라미터 분리가 개선되었으나 Input DTO 활용을 권장합니다.
AuthRequest(웹 계층 DTO)를 직접 받지 않고 개별 파라미터로 분리한 것은 계층 간 결합도를 낮추는 좋은 변경입니다.다만 PR 목표가 Input/Result DTO 패턴을 도입하는 것이고, 이미
SignUpInput이 존재하는 것으로 보이므로, 개별 파라미터 4개 대신SignUpInput을 받는 것이 더 나을 수 있습니다:Member createUser(Credential credential, SignUpInput input);이렇게 하면:
- 파라미터 개수 감소로 메서드 시그니처가 간결해집니다
- 향후 필드 추가 시 시그니처 변경이 불필요합니다
- Input DTO 네이밍 컨벤션과 일관성이 유지됩니다
src/main/java/starlight/adapter/member/auth/security/handler/JwtAccessDeniedHandler.java (1)
23-29: 중복 코드 추출을 고려해볼 수 있습니다.
JwtAuthenticationHandler,JwtAccessDeniedHandler,ExceptionFilter세 핸들러가 유사한 JSON 응답 작성 로직을 공유합니다. 공통 유틸리티 메서드로 추출하면 유지보수성이 향상될 수 있습니다.다만 각 핸들러가 독립적으로 동작하는 것도 장점이므로, 필수는 아닙니다.
🔎 추출 가능한 공통 로직
public class SecurityResponseWriter { public static void writeErrorResponse( HttpServletResponse response, ObjectMapper objectMapper, int status, GlobalErrorType errorType ) throws IOException { if (response.isCommitted()) { return; } response.setContentType("application/json;charset=UTF-8"); response.setStatus(status); ApiResponse<?> errorResponse = ApiResponse.error(errorType); response.getWriter().write(objectMapper.writeValueAsString(errorResponse)); } }src/main/java/starlight/application/member/required/MemberQueryPort.java (1)
13-15: 중복 가능성 검토 제안
findByProviderAndProviderId와findByProviderAndProviderIdOrThrow가 공존하는 구조입니다. 사용 빈도가 낮은 경우, default method를 활용하여 중복을 줄일 수 있습니다:default Member findByProviderAndProviderIdOrThrow(String provider, String providerId) { return findByProviderAndProviderId(provider, providerId) .orElseThrow(() -> new MemberException(...)); }단, 예외 처리 로직이 복잡하거나 adapter에서 최적화가 필요한 경우 현재 구조가 더 적합할 수 있습니다.
개발가이드.md (1)
45-50: 네이밍 예시 추가 제안실패 동작 네이밍 규칙이 잘 정리되어 있습니다. 다만 부정적인 예시(anti-pattern)도 함께 제시하면 더 명확할 수 있습니다:
- 실패 동작을 이름으로 드러낸다. - Optional/nullable 반환: `findById`, `findByEmail` - 예외를 던짐: `findByIdOrThrow`, `findByTokenOrThrow` - 연관 강제 로딩: `findByIdWithCareersAndTags`, `fetchExpertsWithTagsByIds` - ❌ 피해야 할 패턴: `getOrThrow`, `getMember` (조회 동작은 find*로 통일)src/main/java/starlight/application/expertApplication/event/FeedbackRequestEventListener.java (1)
43-52: 실패 처리 전략 구현을 고려하세요.재시도가 모두 실패한 경우의 복구 로직이 TODO로 남아있습니다. TODO에 언급된 세 가지 전략(DLQ 저장, 관리자 알림, DB 실패 기록) 중 최소한 하나는 구현하는 것이 프로덕션 안정성을 위해 권장됩니다.
실패 처리 전략 구현을 도와드릴까요? 예를 들어 실패 이벤트를 DB에 기록하는 간단한 구현부터 시작할 수 있습니다.
src/main/java/starlight/adapter/expertReport/persistence/ExpertReportRepository.java (1)
16-24: 명시적 @query 어노테이션이 불필요할 수 있습니다.현재 JPQL 쿼리는 Spring Data JPA의 메서드 이름 기반 쿼리 파생으로 자동 생성 가능합니다. 예를 들어
findByToken과findAllByBusinessPlanIdOrderByCreatedAtDesc메서드 이름만으로도 동일한 쿼리가 생성됩니다.명시적 @query를 사용하는 특별한 이유(성능 튜닝, 커스텀 조인 전략 등)가 없다면, 유지보수를 위해 메서드 이름 기반 쿼리로 단순화하는 것을 고려해보세요. 다만 "WithComments" 접미사를 통해 EntityGraph 사용을 명시하려는 의도라면 현재 구현도 합리적입니다.
🔎 메서드 이름 기반 쿼리로 단순화하는 경우
@EntityGraph(attributePaths = {"comments"}) - @Query("select er from ExpertReport er where er.token = :token") - Optional<ExpertReport> findByTokenWithComments(@Param("token") String token); + Optional<ExpertReport> findByTokenWithComments(String token); @EntityGraph(attributePaths = {"comments"}) - @Query("select er from ExpertReport er where er.businessPlanId = :businessPlanId order by er.createdAt desc") - List<ExpertReport> findAllByBusinessPlanIdWithCommentsOrderByCreatedAtDesc( - @Param("businessPlanId") Long businessPlanId - ); + List<ExpertReport> findAllByBusinessPlanIdWithCommentsOrderByCreatedAtDesc(Long businessPlanId);src/main/java/starlight/application/member/MemberQueryService.java (1)
21-33: Command 작업이 Query Service에 포함되어 있습니다.
createUser메서드는 데이터를 생성하고 저장하는 Command 작업입니다. 현재 클래스명이MemberQueryService이므로 CQRS 원칙상 읽기 전용 작업만 포함해야 합니다.
createUser메서드를 별도의MemberCommandService로 이동하거나, 서비스 이름을MemberService로 변경하여 Query/Command를 모두 처리하는 서비스임을 명확히 하는 것을 권장합니다.🔎 MemberCommandService로 분리하는 예시
MemberCommandService.java를 새로 생성:
@Service @RequiredArgsConstructor public class MemberCommandService implements MemberCommandUseCase { private final MemberQueryPort memberQueryPort; private final MemberCommandPort memberCommandPort; public Member createUser(Credential credential, String name, String email, String phoneNumber) { memberQueryPort.findByEmail(email).ifPresent(existingUser -> { throw new MemberException(MemberErrorType.MEMBER_ALREADY_EXISTS); }); Member member = Member.create(name, email, phoneNumber, MemberType.FOUNDER, credential, null); return memberCommandPort.save(member); } }그리고 MemberQueryService에서는 제거:
@Service @RequiredArgsConstructor public class MemberQueryService implements MemberQueryUseCase { - private final MemberQueryPort memberQueryPort; - private final MemberCommandPort memberCommandPort; + private final MemberQueryPort memberQueryPort; - public Member createUser(Credential credential, String name, String email, String phoneNumber) { - memberQueryPort.findByEmail(email).ifPresent(existingUser -> { - throw new MemberException(MemberErrorType.MEMBER_ALREADY_EXISTS); - }); - Member member = Member.create(name, email, phoneNumber, MemberType.FOUNDER, credential, null); - return memberCommandPort.save(member); - } - public Member getUserByEmail(String email) { return memberQueryPort.findByEmail(email) .orElseThrow(() -> new MemberException(MemberErrorType.MEMBER_NOT_FOUND)); }src/main/java/starlight/adapter/member/auth/webapi/AuthTokenResolver.java (3)
27-45: 토큰 추출 로직이 견고하게 구현되었습니다.
extractToken과normalizePrefix메서드 모두 null/blank 값과 엣지 케이스를 적절히 처리하고 있습니다.Line 32에서 전체 문자열을 소문자로 변환한 후 prefix를 체크하는 것은 안전하지만, 약간의 성능 개선 여지가 있습니다. 하지만 이는 nitpick 수준이며 현재 구현도 충분히 명확합니다.
🔎 미세 최적화 제안 (선택적)
전체 문자열 변환 대신 regionMatches를 사용할 수 있습니다:
private String extractToken(String raw) { if (raw == null) { return null; } String trimmed = raw.trim(); - if (trimmed.toLowerCase().startsWith(bearerPrefix)) { + if (trimmed.regionMatches(true, 0, bearerPrefix, 0, bearerPrefix.length())) { String token = trimmed.substring(bearerPrefix.length()).trim(); return token.isEmpty() ? null : token; } return trimmed.isEmpty() ? null : trimmed; }하지만 가독성 측면에서 현재 구현이 더 명확하므로, 성능이 병목이 되지 않는 한 현재대로 유지하는 것을 권장합니다.
19-25: 메서드 중복 제거를 위해 공통 로직을 private 메서드로 추출하세요.
resolveAccessToken과resolveRefreshToken이 완전히 동일한 구현을 가지고 있습니다. 현재 코드는 같은 HTTP 헤더(authHeaderName)에서 토큰을 추출하며, 이는 프로젝트의 인증 설계에 따른 의도적 선택으로 보입니다. 토큰들은 HTTP 헤더 위치가 아닌 JWT 페이로드 내용으로 구분됩니다.코드 중복을 제거하기 위해
resolveToken(HttpServletRequest request)같은 private 메서드를 만들고 두 public 메서드에서 호출하는 방식을 권장합니다.예시
public String resolveAccessToken(HttpServletRequest request) { - return extractToken(request.getHeader(authHeaderName)); + return resolveToken(request); } public String resolveRefreshToken(HttpServletRequest request) { - return extractToken(request.getHeader(authHeaderName)); + return resolveToken(request); +} + +private String resolveToken(HttpServletRequest request) { + return extractToken(request.getHeader(authHeaderName)); }
1-46: Deprecated 메서드 선언을 정리하여 리팩토링을 완료하세요.좋은 책임 분리입니다.
AuthTokenResolver로의 마이그레이션이 프로덕션 코드에서 완료되었으며,AuthController와JwtFilter모두 새 구현을 사용하고 있습니다.다음 단계로 리팩토링을 완료하세요:
TokenProvider인터페이스에서 deprecated된resolveAccessToken(),resolveRefreshToken()메서드 선언 제거JwtTokenProvider클래스에서 deprecated 메서드 구현 제거JwtTokenProviderTest에서 deprecated 메서드를 직접 테스트하는 코드 정리
normalizePrefix메서드는 두 클래스에 각각 존재하지만, 내부 유틸리티로 사용되므로 현재 상태로도 무방합니다.src/test/java/starlight/application/businessplan/BusinessPlanServiceImplIntegrationTest.java (1)
52-79: MemberQueryPort 테스트 구현이 적절합니다.포트 기반 아키텍처로의 전환에 맞춰 테스트 빈 설정이 올바르게 업데이트되었습니다. 새로운 메서드들(
findByEmail,findByProviderAndProviderId,findByProviderAndProviderIdOrThrow)이 인터페이스 요구사항에 맞게 구현되어 있습니다.다만, 테스트 클래스 내에서
java.util.Optional을 직접 fully-qualified로 사용하고 있는데, 상단에 import를 추가하면 더 깔끔해집니다.🔎 제안: Optional import 추가
import starlight.application.member.required.MemberQueryPort; import starlight.domain.businessplan.entity.BusinessPlan; import starlight.domain.businessplan.entity.SubSection; import starlight.domain.businessplan.enumerate.SubSectionType; import starlight.domain.member.entity.Member; import java.util.List; +import java.util.Optional;그리고 메서드 내에서:
@Override -public java.util.Optional<Member> findByEmail(String email) { - return java.util.Optional.empty(); +public Optional<Member> findByEmail(String email) { + return Optional.empty(); }src/main/java/starlight/application/expertReport/ExpertReportService.java (1)
100-121: 리스트 조회 메서드 리팩토링 확인
- 반환 타입이
List<ExpertReportWithExpertResult>로 변경businessPlanQuery.findByIdOrThrow(businessPlanId)호출 후 결과를 사용하지 않음 - 존재 여부 검증용으로 보이나, 명시적인 주석이 있으면 의도가 더 명확해집니다.🔎 의도 명확화를 위한 제안
@Transactional(readOnly = true) public List<ExpertReportWithExpertResult> getExpertReportsWithExpertByBusinessPlanId(Long businessPlanId) { + // 사업계획서 존재 여부 검증 businessPlanQuery.findByIdOrThrow(businessPlanId);src/main/java/starlight/adapter/order/toss/TossClient.java (1)
18-51: PaymentGatewayPort 구현 및 에러 핸들링포트 기반 아키텍처가 잘 적용되었습니다. 다만
catch (Exception e)블록이 모든 예외를 포괄적으로 처리하고 있어, 특정 예외 유형(예:RestClientException,IllegalStateException)에 따른 세분화된 처리가 필요할 수 있습니다.현재 구현에서는 validation 실패도
TOSS_CLIENT_CONFIRM_ERROR로 래핑되어 디버깅이 어려울 수 있습니다.🔎 예외 처리 세분화 제안
} catch (Exception e) { log.error("토스 결제 승인 요청 중 에러 발생. orderId: {}, paymentKey: {}, message: {}", orderCode, paymentKey, e.getMessage(), e); + if (e instanceof IllegalStateException) { + throw new OrderException(OrderErrorType.TOSS_CLIENT_VALIDATION_ERROR); + } throw new OrderException(OrderErrorType.TOSS_CLIENT_CONFIRM_ERROR); }src/main/java/starlight/adapter/member/auth/security/oauth2/CustomOAuth2UserService.java (1)
42-54:orElseGet내부에서 부작용(save)을 수행하는 것은 권장되지 않습니다.
orElseGet은 값의 지연 생성을 위한 것이지, 부작용(side effect)을 수행하기 위한 것이 아닙니다. 코드 가독성과 의도 명확성을 위해 분리하는 것을 고려해 주세요.🔎 제안하는 리팩토링
- Member member = found.orElseGet(() -> - memberCommandPort.save( - Member.newSocial( - parsed.name(), - parsed.email(), - parsed.provider(), - parsed.providerId(), - null, - MemberType.FOUNDER, - parsed.profileImageUrl() - ) - ) - ); + Member member; + if (found.isPresent()) { + member = found.get(); + } else { + Member newMember = Member.newSocial( + parsed.name(), + parsed.email(), + parsed.provider(), + parsed.providerId(), + null, + MemberType.FOUNDER, + parsed.profileImageUrl() + ); + member = memberCommandPort.save(newMember); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (109)
src/main/java/starlight/adapter/aireport/webapi/AiReportController.javasrc/main/java/starlight/adapter/aireport/webapi/ImageController.javasrc/main/java/starlight/adapter/aireport/webapi/swagger/ImageApiDoc.javasrc/main/java/starlight/adapter/auth/security/handler/JwtAccessDeniedHandler.javasrc/main/java/starlight/adapter/auth/security/handler/JwtAuthenticationHandler.javasrc/main/java/starlight/adapter/auth/security/jwt/dto/TokenResponse.javasrc/main/java/starlight/adapter/auth/webapi/AuthController.javasrc/main/java/starlight/adapter/auth/webapi/swagger/AuthApiDoc.javasrc/main/java/starlight/adapter/businessplan/persistence/BusinessPlanJpa.javasrc/main/java/starlight/adapter/businessplan/webapi/BusinessPlanController.javasrc/main/java/starlight/adapter/expert/persistence/ExpertJpa.javasrc/main/java/starlight/adapter/expertApplication/email/SMTPEmailSender.javasrc/main/java/starlight/adapter/expertApplication/webapi/ExpertApplicationController.javasrc/main/java/starlight/adapter/expertApplication/webapi/swagger/ExpertApplicationApiDoc.javasrc/main/java/starlight/adapter/expertReport/persistence/ExpertReportJpa.javasrc/main/java/starlight/adapter/expertReport/persistence/ExpertReportRepository.javasrc/main/java/starlight/adapter/expertReport/webapi/ExpertReportController.javasrc/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportCareerResponse.javasrc/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportExpertResponse.javasrc/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportResponse.javasrc/main/java/starlight/adapter/member/auth/redis/RedisKeyValueMap.javasrc/main/java/starlight/adapter/member/auth/security/auth/AuthDetails.javasrc/main/java/starlight/adapter/member/auth/security/auth/AuthDetailsService.javasrc/main/java/starlight/adapter/member/auth/security/filter/ExceptionFilter.javasrc/main/java/starlight/adapter/member/auth/security/filter/JwtFilter.javasrc/main/java/starlight/adapter/member/auth/security/handler/JwtAccessDeniedHandler.javasrc/main/java/starlight/adapter/member/auth/security/handler/JwtAuthenticationHandler.javasrc/main/java/starlight/adapter/member/auth/security/jwt/JwtTokenProvider.javasrc/main/java/starlight/adapter/member/auth/security/jwt/dto/TokenResponse.javasrc/main/java/starlight/adapter/member/auth/security/oauth2/CustomOAuth2UserService.javasrc/main/java/starlight/adapter/member/auth/security/oauth2/OAuth2Attributes.javasrc/main/java/starlight/adapter/member/auth/security/oauth2/OAuth2SuccessHandler.javasrc/main/java/starlight/adapter/member/auth/webapi/AuthController.javasrc/main/java/starlight/adapter/member/auth/webapi/AuthTokenResolver.javasrc/main/java/starlight/adapter/member/auth/webapi/dto/request/AuthRequest.javasrc/main/java/starlight/adapter/member/auth/webapi/dto/request/SignInRequest.javasrc/main/java/starlight/adapter/member/auth/webapi/dto/response/MemberResponse.javasrc/main/java/starlight/adapter/member/auth/webapi/swagger/AuthApiDoc.javasrc/main/java/starlight/adapter/member/persistence/CredentialRepository.javasrc/main/java/starlight/adapter/member/persistence/MemberJpa.javasrc/main/java/starlight/adapter/member/webapi/MemberController.javasrc/main/java/starlight/adapter/member/webapi/dto/MemberDetailResponse.javasrc/main/java/starlight/adapter/member/webapi/swagger/MemberApiDoc.javasrc/main/java/starlight/adapter/order/toss/TossClient.javasrc/main/java/starlight/adapter/order/webapi/OrderController.javasrc/main/java/starlight/adapter/order/webapi/dto/response/OrderCancelResponse.javasrc/main/java/starlight/adapter/order/webapi/swagger/OrderApiDoc.javasrc/main/java/starlight/application/aireport/AiReportServiceImpl.javasrc/main/java/starlight/application/auth/AuthServiceImpl.javasrc/main/java/starlight/application/auth/provided/AuthService.javasrc/main/java/starlight/application/auth/required/TokenProvider.javasrc/main/java/starlight/application/businessplan/BusinessPlanServiceImpl.javasrc/main/java/starlight/application/businessplan/required/BusinessPlanQuery.javasrc/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.javasrc/main/java/starlight/application/expertApplication/ExpertApplicationQueryService.javasrc/main/java/starlight/application/expertApplication/event/FeedbackRequestEventListener.javasrc/main/java/starlight/application/expertApplication/event/FeedbackRequestInput.javasrc/main/java/starlight/application/expertApplication/required/EmailSender.javasrc/main/java/starlight/application/expertApplication/required/ExpertLookupPort.javasrc/main/java/starlight/application/expertReport/ExpertReportService.javasrc/main/java/starlight/application/expertReport/provided/ExpertReportServiceUseCase.javasrc/main/java/starlight/application/expertReport/provided/dto/ExpertReportWithExpertResult.javasrc/main/java/starlight/application/expertReport/required/ExpertReportQueryPort.javasrc/main/java/starlight/application/member/CredentialServiceImpl.javasrc/main/java/starlight/application/member/MemberQueryService.javasrc/main/java/starlight/application/member/MemberServiceImpl.javasrc/main/java/starlight/application/member/auth/AuthServiceImpl.javasrc/main/java/starlight/application/member/auth/provided/AuthUseCase.javasrc/main/java/starlight/application/member/auth/provided/dto/AuthMemberResult.javasrc/main/java/starlight/application/member/auth/provided/dto/AuthTokenResult.javasrc/main/java/starlight/application/member/auth/provided/dto/SignInInput.javasrc/main/java/starlight/application/member/auth/provided/dto/SignUpInput.javasrc/main/java/starlight/application/member/auth/required/KeyValueMap.javasrc/main/java/starlight/application/member/auth/required/TokenProvider.javasrc/main/java/starlight/application/member/provided/CredentialService.javasrc/main/java/starlight/application/member/provided/MemberQueryUseCase.javasrc/main/java/starlight/application/member/required/MemberCommandPort.javasrc/main/java/starlight/application/member/required/MemberQueryPort.javasrc/main/java/starlight/application/order/OrderPaymentService.javasrc/main/java/starlight/application/order/provided/OrderPaymentServiceUseCase.javasrc/main/java/starlight/application/order/provided/dto/PaymentHistoryItemResult.javasrc/main/java/starlight/application/order/provided/dto/TossClientResult.javasrc/main/java/starlight/application/order/required/PaymentGatewayPort.javasrc/main/java/starlight/bootstrap/SecurityConfig.javasrc/main/java/starlight/domain/member/auth/exception/AuthErrorType.javasrc/main/java/starlight/domain/member/auth/exception/AuthException.javasrc/main/java/starlight/shared/auth/AuthenticatedMember.javasrc/test/java/starlight/adapter/aireport/infrastructure/webapi/ImageControllerIntegrationTest.javasrc/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.javasrc/test/java/starlight/adapter/member/auth/redis/RedisKeyValueMapTest.javasrc/test/java/starlight/adapter/member/auth/security/filter/ExceptionFilterUnitTest.javasrc/test/java/starlight/adapter/member/auth/security/jwt/JwtTokenProviderTest.javasrc/test/java/starlight/adapter/member/auth/security/oauth2/CustomOAuth2UserServiceUnitTest.javasrc/test/java/starlight/adapter/member/auth/security/oauth2/OAuth2AttributesUnitTest.javasrc/test/java/starlight/adapter/member/auth/security/oauth2/OAuth2LoginFlowTest.javasrc/test/java/starlight/adapter/member/auth/security/oauth2/OAuth2SuccessHandlerUnitTest.javasrc/test/java/starlight/adapter/member/auth/security/oauth2/TestOAuth2Objects.javasrc/test/java/starlight/adapter/member/auth/webapi/AuthControllerSliceTest.javasrc/test/java/starlight/application/aireport/AiReportServiceImplUnitTest.javasrc/test/java/starlight/application/businessplan/BusinessPlanServiceImplIntegrationTest.javasrc/test/java/starlight/application/businessplan/BusinessPlanServiceImplUnitTest.javasrc/test/java/starlight/application/member/CredentialServiceImplIntegrationTest.javasrc/test/java/starlight/application/member/CredentialServiceImplUnitTest.javasrc/test/java/starlight/application/member/MemberQueryServiceIntegrationTest.javasrc/test/java/starlight/application/member/MemberQueryServiceUnitTest.javasrc/test/java/starlight/application/member/auth/AuthServiceImplIntegrationTest.javasrc/test/java/starlight/application/member/auth/AuthServiceImplUnitTest.javasrc/test/java/starlight/bootstrap/SecurityConfigTest.java개발가이드.md
💤 Files with no reviewable changes (10)
- src/main/java/starlight/application/auth/required/TokenProvider.java
- src/main/java/starlight/adapter/auth/webapi/swagger/AuthApiDoc.java
- src/main/java/starlight/adapter/auth/security/handler/JwtAuthenticationHandler.java
- src/main/java/starlight/adapter/auth/webapi/AuthController.java
- src/main/java/starlight/application/auth/AuthServiceImpl.java
- src/main/java/starlight/application/auth/provided/AuthService.java
- src/main/java/starlight/adapter/member/persistence/CredentialRepository.java
- src/main/java/starlight/adapter/auth/security/handler/JwtAccessDeniedHandler.java
- src/main/java/starlight/adapter/auth/security/jwt/dto/TokenResponse.java
- src/main/java/starlight/application/member/MemberServiceImpl.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-26T15:38:55.886Z
Learnt from: SeongHo5356
Repo: StartUpLight/STARLIGHT_BE PR: 67
File: src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java:36-51
Timestamp: 2025-12-26T15:38:55.886Z
Learning: StarLight의 Expert 조회에서 `findByIdWithCareersAndTags()` 메서드는 단건 상세 조회용으로, ExpertDetailResponse가 categories 필드를 포함하지 않기 때문에 의도적으로 categories 페치를 제외합니다. ExpertListResponse만 categories를 포함합니다.
Applied to files:
src/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportExpertResponse.java
🧬 Code graph analysis (15)
src/main/java/starlight/adapter/member/webapi/swagger/MemberApiDoc.java (1)
src/main/java/starlight/adapter/expertApplication/webapi/swagger/ExpertApplicationApiDoc.java (1)
Schema(306-324)
src/main/java/starlight/application/member/MemberQueryService.java (1)
src/main/java/starlight/domain/member/exception/MemberException.java (1)
MemberException(6-11)
src/main/java/starlight/application/member/auth/AuthServiceImpl.java (5)
src/main/java/starlight/application/member/MemberQueryService.java (1)
Service(14-49)src/main/java/starlight/adapter/member/auth/security/auth/AuthDetailsService.java (1)
Service(14-29)src/main/java/starlight/application/member/CredentialServiceImpl.java (1)
Service(12-41)src/main/java/starlight/domain/member/auth/exception/AuthException.java (1)
AuthException(6-11)src/main/java/starlight/domain/member/exception/MemberException.java (1)
MemberException(6-11)
src/main/java/starlight/adapter/member/persistence/MemberJpa.java (2)
src/main/java/starlight/domain/member/exception/MemberException.java (1)
MemberException(6-11)src/main/java/starlight/adapter/businessplan/persistence/BusinessPlanJpa.java (1)
Repository(12-39)
src/main/java/starlight/adapter/member/auth/security/jwt/dto/TokenResponse.java (1)
src/main/java/starlight/adapter/auth/security/jwt/dto/TokenResponse.java (1)
TokenResponse(3-11)
src/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportExpertResponse.java (1)
src/main/java/starlight/adapter/expert/webapi/dto/ExpertListResponse.java (1)
ExpertListResponse(8-57)
src/test/java/starlight/application/member/CredentialServiceImplUnitTest.java (1)
src/main/java/starlight/domain/member/auth/exception/AuthException.java (1)
AuthException(6-11)
src/main/java/starlight/adapter/member/auth/security/handler/JwtAuthenticationHandler.java (6)
src/main/java/starlight/adapter/member/auth/security/jwt/JwtTokenProvider.java (1)
Component(25-227)src/main/java/starlight/adapter/member/auth/security/handler/JwtAccessDeniedHandler.java (1)
Component(15-31)src/main/java/starlight/adapter/member/auth/webapi/AuthTokenResolver.java (1)
Component(7-46)src/main/java/starlight/adapter/member/auth/redis/RedisKeyValueMap.java (1)
Component(13-79)src/main/java/starlight/adapter/member/auth/security/filter/ExceptionFilter.java (1)
Component(17-49)src/main/java/starlight/adapter/auth/security/handler/JwtAuthenticationHandler.java (1)
Component(12-20)
src/test/java/starlight/application/member/CredentialServiceImplIntegrationTest.java (1)
src/test/java/starlight/application/member/CredentialServiceImplUnitTest.java (1)
ExtendWith(16-53)
src/main/java/starlight/adapter/member/auth/security/auth/AuthDetailsService.java (3)
src/main/java/starlight/application/member/auth/AuthServiceImpl.java (1)
Service(23-147)src/main/java/starlight/adapter/member/auth/security/oauth2/CustomOAuth2UserService.java (1)
Service(18-65)src/main/java/starlight/application/member/CredentialServiceImpl.java (1)
Service(12-41)
src/main/java/starlight/application/member/CredentialServiceImpl.java (1)
src/main/java/starlight/domain/member/auth/exception/AuthException.java (1)
AuthException(6-11)
src/test/java/starlight/adapter/member/auth/webapi/AuthControllerSliceTest.java (2)
src/test/java/starlight/bootstrap/SecurityConfigTest.java (1)
WebMvcTest(22-44)src/test/java/starlight/adapter/member/auth/security/oauth2/OAuth2LoginFlowTest.java (1)
TestConfiguration(61-80)
src/test/java/starlight/application/member/auth/AuthServiceImplUnitTest.java (1)
src/main/java/starlight/domain/member/auth/exception/AuthException.java (1)
AuthException(6-11)
src/main/java/starlight/adapter/member/webapi/MemberController.java (3)
src/main/java/starlight/adapter/member/auth/security/oauth2/OAuth2SuccessHandler.java (1)
Slf4j(20-51)src/main/java/starlight/adapter/member/auth/security/filter/JwtFilter.java (1)
Slf4j(22-50)src/main/java/starlight/bootstrap/SecurityConfig.java (1)
Slf4j(34-127)
src/test/java/starlight/application/member/auth/AuthServiceImplIntegrationTest.java (1)
src/main/java/starlight/domain/member/auth/exception/AuthException.java (1)
AuthException(6-11)
🪛 Gitleaks (8.30.0)
src/main/java/starlight/adapter/member/auth/webapi/swagger/AuthApiDoc.java
[high] 113-113: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 114-114: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 266-266: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 267-267: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
src/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportResponse.java
Outdated
Show resolved
Hide resolved
Test Results248 tests 248 ✅ 10s ⏱️ Results for commit 9eeae3b. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/starlight/adapter/businessplan/persistence/BusinessPlanJpa.java (1)
26-30: 네이밍 일관성을 위해 메서드명 변경을 권장합니다.Line 19에서
getOrThrow를findByIdOrThrow로 변경했지만, 이 메서드는 여전히get접두사를 사용하고 있습니다. 동일한 클래스 내에서 유사한 목적(필수 조회 + 예외 발생)을 가진 메서드들의 네이밍을 통일하는 것이 좋습니다.
findByIdWithAllSubSectionsOrThrow또는findWithAllSubSectionsByIdOrThrow와 같이find접두사로 시작하도록 변경을 고려해보세요.src/main/java/starlight/application/businessplan/required/BusinessPlanQuery.java (1)
11-11: 인터페이스 전체의 네이밍 일관성을 위한 변경을 권장합니다.Line 9의 메서드는
find접두사로 변경되었지만, 이 메서드는 여전히get접두사를 사용하고 있습니다. 포트 인터페이스 내의 모든 조회 메서드가 동일한 네이밍 패턴을 따르도록findByIdWithAllSubSectionsOrThrow형태로 변경하는 것을 고려해보세요.이 변경은 구현체인
BusinessPlanJpa클래스에도 동일하게 적용되어야 합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/starlight/adapter/businessplan/persistence/BusinessPlanJpa.javasrc/main/java/starlight/application/businessplan/BusinessPlanServiceImpl.javasrc/main/java/starlight/application/businessplan/required/BusinessPlanQuery.java
⏰ 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). (2)
- GitHub Check: build-and-push
- GitHub Check: test
🔇 Additional comments (4)
src/main/java/starlight/adapter/businessplan/persistence/BusinessPlanJpa.java (1)
19-23: 메서드명 변경이 적절합니다!
getOrThrow에서findByIdOrThrow로의 변경은 Spring Data JPA의 네이밍 컨벤션과 잘 맞으며, 메서드의 의도를 더 명확하게 표현합니다. 'find' 접두사는 조회 작업을, 'ById'는 조회 기준을, 'OrThrow'는 실패 시 예외 발생을 명시적으로 나타냅니다.src/main/java/starlight/application/businessplan/required/BusinessPlanQuery.java (1)
9-9: 인터페이스 메서드명 변경이 적절합니다!포트 인터페이스의 메서드명을
findByIdOrThrow로 변경한 것은 헥사고날 아키텍처에서 명확한 계약을 정의하며, 구현체들과의 일관성을 보장합니다.src/main/java/starlight/application/businessplan/BusinessPlanServiceImpl.java (2)
19-19: 포트 네이밍 규칙이 잘 적용되었습니다!
MemberQuery에서MemberQueryPort로의 변경은 헥사고날 아키텍처의 포트 명명 규칙을 명확하게 따르고 있습니다. 'Port' 접미사는 이 인터페이스가 애플리케이션 계층의 포트임을 명시적으로 나타내며, 어댑터와의 경계를 분명하게 합니다.Also applies to: 38-38
44-44: 메서드 호출이 올바르게 업데이트되었습니다!두 곳 모두에서
findByIdOrThrow메서드 호출로 일관되게 변경되었습니다. 서비스 레이어 전반에 걸쳐 새로운 네이밍 컨벤션이 적절하게 적용되어 코드의 가독성과 일관성이 향상되었습니다.Also applies to: 239-239
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/starlight/application/member/auth/AuthServiceImpl.java (1)
74-93: 로그아웃 로직은 정확하나, 코드 중복을 제거하면 더 깔끔해집니다.refreshToken과 accessToken에 대한 검증 로직이 동일하게 반복되고 있습니다(lines 86-91). 헬퍼 메서드를 추가하거나 단일 조건문으로 통합하면 가독성이 향상됩니다.
🔎 중복 제거 제안
@Override @Transactional public void signOut(String refreshToken, String accessToken) { if (refreshToken == null || accessToken == null) { throw new AuthException(AuthErrorType.TOKEN_NOT_FOUND); } - if (!tokenProvider.validateToken(accessToken)) { - throw new AuthException(AuthErrorType.TOKEN_INVALID); - } - if (!tokenProvider.validateToken(refreshToken)) { + if (!tokenProvider.validateToken(accessToken) || !tokenProvider.validateToken(refreshToken)) { throw new AuthException(AuthErrorType.TOKEN_INVALID); } tokenProvider.logoutTokens(refreshToken, accessToken); }src/main/java/starlight/application/member/CredentialServiceImpl.java (1)
18-27: 과거 리뷰 피드백이 완벽하게 반영되었습니다! 우수한 리팩토링입니다.이전 리뷰에서 지적된 Javadoc과 실제 동작의 불일치 문제가 완전히 해결되었습니다:
- Javadoc이 "생성하는 메서드"로 정확하게 수정됨
- "@return 저장되지 않은 Credential"을 명시하여 메서드의 책임 범위를 명확히 함
- 저장 로직을 제거하여 단일 책임 원칙(SRP)을 준수
AuthRequest대신String rawPassword를 직접 받아 결합도 감소이는 헥사고날 아키텍처에서 application 서비스가 비즈니스 로직(암호화)만 담당하고, 영속성 책임은 상위 유스케이스로 분리하는 올바른 설계입니다.
선택적 개선 사항: 입력 검증 추가 고려
rawPassword에 대한 null 또는 빈 문자열 검증을 추가하면 방어적 프로그래밍을 강화할 수 있습니다. 다만 상위 레이어(컨트롤러/유스케이스)에서 이미 검증하고 있다면 불필요할 수 있습니다.🔎 선택적 개선안
public Credential createCredential(String rawPassword) { + if (rawPassword == null || rawPassword.isBlank()) { + throw new AuthException(AuthErrorType.INVALID_PASSWORD); + } String hashedPassword = passwordEncoder.encode(rawPassword); return Credential.create(hashedPassword); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
configsrc/main/java/starlight/application/member/CredentialServiceImpl.javasrc/main/java/starlight/application/member/auth/AuthServiceImpl.javasrc/test/java/starlight/adapter/member/auth/security/jwt/JwtTokenProviderTest.javasrc/test/java/starlight/application/businessplan/BusinessPlanServiceImplUnitTest.java
✅ Files skipped from review due to trivial changes (1)
- config
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/starlight/application/businessplan/BusinessPlanServiceImplUnitTest.java
- src/test/java/starlight/adapter/member/auth/security/jwt/JwtTokenProviderTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/starlight/application/member/CredentialServiceImpl.java (1)
src/main/java/starlight/domain/member/auth/exception/AuthException.java (1)
AuthException(6-11)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: build-and-push
🔇 Additional comments (7)
src/main/java/starlight/application/member/auth/AuthServiceImpl.java (5)
23-34: 클래스 구조와 의존성 설정이 우수합니다.헥사고날 아키텍처의 포트/어댑터 패턴이 잘 적용되었고, 생성자 주입 방식으로 의존성을 관리하고 있습니다. 토큰 만료 시간을 외부 설정으로 관리하는 점도 좋습니다.
36-54: 이전 리뷰 피드백이 반영되었습니다. 잘 구현되었습니다.Javadoc의 파라미터명이 실제 메서드 시그니처(
input)와 일치하도록 수정되었고, 회원가입 로직이 명확하게 구현되어 있습니다. Credential 생성 → Member 생성 → Result 반환 흐름이 깔끔합니다.
56-72: 이전 리뷰 피드백이 반영되었습니다. 로그인 로직이 잘 구현되었습니다.Javadoc 파라미터명이 수정되었고, 로그인 시 Redis에 refresh token을 만료 시간과 함께 저장하는 로직이 정확하게 구현되어 있습니다.
95-131: 이전 리뷰 피드백이 반영되었습니다. 토큰 재발급 로직이 철저하게 구현되었습니다.Javadoc 파라미터명(
memberId)이 수정되었고, 다층적인 검증 로직(토큰 유효성, 이메일 일치, Redis 저장값 비교)이 잘 구현되어 있습니다. 보안을 고려한 방어적 프로그래밍이 돋보입니다.참고: Line 125의
refreshToken.isEmpty()체크는extractToken메서드가 이미 빈 토큰을 거부하므로 기술적으로 불필요하지만, 명시적 방어 코드로서 유지해도 무방합니다.
133-146: 토큰 추출 헬퍼 메서드가 잘 구현되었습니다.Bearer 접두어 처리와 공백 제거, 빈 토큰 검증 등 엣지 케이스를 모두 고려한 방어적 프로그래밍이 적용되어 있습니다.
src/main/java/starlight/application/member/CredentialServiceImpl.java (2)
7-8: 도메인 재배치가 올바르게 반영되었습니다.인증 관련 예외를
starlight.domain.member.auth.exception패키지로 이동한 것은 PR의 목표인 도메인 책임 재배치와 일치하며, 헥사고날 아키텍처에서 application 레이어가 도메인 예외를 사용하는 것은 적절한 의존성 방향입니다.
34-40: 변경사항 없음 - 로직 정상 확인
checkPassword메서드는 이번 리팩토링에서 변경되지 않았으며, 비밀번호 검증 로직이 정확하게 구현되어 있습니다.AuthException사용도 업데이트된 import 경로와 일관됩니다.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportResponse.java (1)
21-31: LGTM! 이전 리뷰 피드백이 반영되었습니다.
fromEntities()메서드가 이제applicationCount를 파라미터로 받아ExpertDetailResult.from(expert, applicationCount)에 전달합니다. 이전에 하드코딩되어 있던0L문제가 해결되었습니다.
🧹 Nitpick comments (1)
src/main/java/starlight/application/expert/ExpertDetailQueryService.java (1)
21-21: LGTM! 새 포트 타입으로 정상 변경되었습니다.타입 변경이 올바르게 적용되었습니다. 필드명도
expertApplicationCountLookupPort로 맞추면 타입과의 일관성이 높아지지만, 현재 상태로도 기능상 문제는 없습니다.🔎 선택적 네이밍 개선
- private final ExpertApplicationCountLookupPort expertApplicationLookupPort; + private final ExpertApplicationCountLookupPort expertApplicationCountLookupPort;사용하는 곳도 함께 변경 필요:
- Map<Long, Long> countMap = expertApplicationLookupPort.countByExpertIds(expertIds); + Map<Long, Long> countMap = expertApplicationCountLookupPort.countByExpertIds(expertIds);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpaPort.javasrc/main/java/starlight/adapter/expertReport/webapi/ExpertReportController.javasrc/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportResponse.javasrc/main/java/starlight/application/expert/ExpertDetailQueryService.javasrc/main/java/starlight/application/expert/required/ExpertApplicationCountLookupPort.javasrc/main/java/starlight/application/expertReport/ExpertReportService.javasrc/main/java/starlight/application/expertReport/provided/dto/ExpertReportWithExpertResult.javasrc/main/java/starlight/application/expertReport/required/ExpertApplicationCountLookupPort.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/starlight/application/expertReport/required/ExpertApplicationCountLookupPort.java (1)
src/main/java/starlight/application/expert/required/ExpertApplicationLookupPort.java (2)
ExpertApplicationLookupPort(6-9)countByExpertIds(8-8)
⏰ 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). (2)
- GitHub Check: build-and-push
- GitHub Check: test
🔇 Additional comments (8)
src/main/java/starlight/application/expertReport/provided/dto/ExpertReportWithExpertResult.java (1)
6-16: LGTM! 깔끔한 Result DTO 구현입니다.
*Result네이밍 컨벤션을 잘 따르고 있으며, 정적 팩토리 메서드of()를 통해 명확한 생성 API를 제공합니다.applicationCount를 포함하여 전문가 지원 개수를 동적으로 전달할 수 있도록 잘 설계되었습니다.src/main/java/starlight/application/expert/required/ExpertApplicationCountLookupPort.java (1)
6-8: 포트 네이밍 개선 - 의미가 명확해졌습니다.
ExpertApplicationLookupPort→ExpertApplicationCountLookupPort로 변경하여 카운트 조회 목적이 명확히 드러납니다. 다만, 앞서 언급한 대로expertReport.required패키지의 동일 인터페이스와 통합이 필요합니다.src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpaPort.java (1)
50-66: LGTM! countByExpertIds 구현이 잘 되어 있습니다.
- null/empty 입력에 대한 방어적 처리
- 일관된 예외 처리 패턴
- 스트림을 활용한 깔끔한 Map 변환
src/main/java/starlight/adapter/expertReport/webapi/ExpertReportController.java (2)
30-42: LGTM! applicationCount가 올바르게 전파됩니다.
ExpertReportWithExpertResult에서applicationCount()를 추출하여fromEntities()로 전달하는 흐름이 잘 구현되었습니다. 이전 리뷰에서 지적된 하드코딩0L문제가 해결되었습니다.
48-54: LGTM! 단건 조회에서도 동일한 패턴 적용
getExpertReport메서드에서도applicationCount가 올바르게 전달되고 있습니다.src/main/java/starlight/application/expertReport/ExpertReportService.java (3)
46-48: LGTM! ExpertApplicationCountLookupPort 의존성 주입카운트 조회를 위한 새 포트가 올바르게 주입되었습니다. 필드명 일관성을 위해
expertApplicationCountLookupPort로 명명하는 것도 고려해볼 수 있습니다.
91-101: LGTM! 단건 조회에서 applicationCount 통합
getExpertReportWithExpert메서드에서 전문가 ID 기반으로 지원 건수를 조회하고ExpertReportWithExpertResult에 포함시키는 로직이 잘 구현되었습니다.
105-130: LGTM! 다건 조회에서도 효율적으로 applicationCount 처리
getExpertReportsWithExpertByBusinessPlanId에서 모든 전문가 ID를 한 번에 조회(countByExpertIds)하여 N+1 문제를 방지하고 있습니다. 배치 조회 패턴이 잘 적용되었습니다.
src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpaPort.java
Show resolved
Hide resolved
src/main/java/starlight/application/expertReport/required/ExpertApplicationCountLookupPort.java
Show resolved
Hide resolved
SeongHo5356
changed the title
SeongHo5356
changed the title
SeongHo5356
changed the title
SeongHo5356
added
✅ TEST
| public ApiResponse<OrderPrepareResponse> prepareOrder( | ||
| @Valid @RequestBody OrderPrepareRequest request, | ||
| @AuthenticationPrincipal AuthDetails authDetails | ||
| @AuthenticationPrincipal AuthenticatedMember authDetails |
There was a problem hiding this comment.
여기서 변수명도 authenticatedMember로 통일해주면 좋을 것 같습니다~~
There was a problem hiding this comment.
다음 pr에 같이 반영해서 올려놓을게!
🚀 Why - 해결하려는 문제가 무엇인가요?
Provider안에 섞여 있어 가독성과 유지보수성이 떨어졌습니다.✅ What - 무엇이 변경됐나요?
AuthTokenResolverTokenProvider(JwtTokenProvider)로 역할 분리*Input규칙으로 정리ApiResponse형태로 정리@AuthenticationPrincipal로 받는 타입을 공용 principal(AuthenticatedMember)로 통일하는 방향으로 정리🛠️ How - 어떻게 해결했나요?
JwtTokenProvider에서 요청 파싱 책임을 제거하고,AuthTokenResolver로 “헤더/쿠키 등에서 토큰 추출”을 위임했습니다.JwtFilter에서tokenResolver.resolveAccessToken(request)로 토큰을 가져온 뒤,tokenProvider.validateToken(token)+redisClient.getValue(token) == null(로그아웃 토큰 여부)로 2중 검증하도록 개선했습니다.JwtAccessDeniedHandler,JwtAuthenticationHandler)가sendError로 흩뿌리던 응답을 ApiResponse JSON 응답으로 통일했습니다.cancel(orderCode, reason)형태로 단순화하여 웹 계층 DTO 결합을 제거했습니다.🖼️ Attachment
💬 기타 코멘트
./gradlew testAuthControllerSliceTest,JwtTokenProviderTest등Summary by CodeRabbit
Refactor
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.