[Suhhee] week08 미션#90
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09ad777067
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }) | ||
| @GetMapping("/v1/reviews/me") | ||
| public ApiResponse<ReviewResponseDto.CursorPagination<ReviewResponseDto.MyReviewDto>> getMyReviews( | ||
| @RequestParam("userId") Long userId, |
There was a problem hiding this comment.
/v1/reviews/me는 SecurityConfig 기준 Private API인데, 현재는 클라이언트가 userId 쿼리 파라미터를 바꾸면 로그인한 사용자와 다른 사용자의 리뷰도 조회할 수 있습니다. Spring Security를 붙인 뒤에는 컨트롤러에서 @AuthenticationPrincipal CustomUserDetails 등으로 인증된 사용자 ID를 얻고, 요청 DTO/파라미터의 사용자 식별자는 제거하는 방향이 좋습니다. 다음 학습 포인트는 인증 주체(Authentication Principal)와 서비스 계층의 소유권 검증입니다.
Useful? React with 👍 / 👎.
| @Query("SELECT r FROM Review r WHERE r.user.id = :userId ORDER BY r.id DESC") | ||
| Slice<Review> findMyReviewsOrderByIdDesc(@Param("userId") Long userId, Pageable pageable); |
There was a problem hiding this comment.
이 JPQL은 Review만 조회하지만, 서비스에서 각 리뷰의 store, user, ownerComment를 바로 접근하므로 LAZY 연관관계마다 추가 select가 발생하는 N+1 구조가 됩니다. 페이지 크기가 10이면 리뷰 조회 1번 뒤에 가게/유저/댓글 조회가 반복되어 JPA 페이징 학습 예제로도 오해가 생길 수 있습니다. @EntityGraph, join fetch 가능한 to-one 관계 fetch, 또는 필요한 필드만 DTO projection으로 가져오는 방법을 적용하고 JPA fetch 전략과 N+1 원인을 함께 정리해 보세요.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| User newUser = User.builder() | ||
| .email(requestDto.email()) |
There was a problem hiding this comment.
회원가입에서 전달된 이메일을 그대로 저장하면 같은 이메일 계정이 여러 개 생길 수 있고, 로그인 시 findByEmail이 단일 사용자를 기대하기 때문에 어떤 계정의 비밀번호를 검증해야 하는지 모호해지거나 조회 예외로 이어질 수 있습니다. 저장 전에 existsByEmail로 도메인 예외를 반환하고, 엔티티/DB에는 unique 제약을 함께 두어 서비스 검증과 데이터 무결성을 같이 보장하는 편이 좋습니다. 다음 학습 포인트는 유니크 제약, 서비스 레벨 검증, 인증 식별자 설계입니다.
Useful? React with 👍 / 👎.
kjhh2605
left a comment
kjhh2605
left a comment
There was a problem hiding this comment.
[키워드 조사]
Spring Security의 필터 체인, Authentication/Authorization, Stateful/Stateless를 핵심 객체 중심으로 정리한 점이 좋습니다. 다만 Stateful/Stateless 설명을 현재 코드의 formLogin 세션 방식, CSRF 비활성화, 향후 JWT 무상태 방식과 연결하면 설정 선택 근거가 더 명확해집니다. 추가로 SecurityContextHolder, UserDetailsService, AuthenticationProvider가 로그인 요청에서 어떤 순서로 호출되는지 흐름도를 함께 조사하는 것을 권장합니다.
[코드 리뷰]
Spring Security 의존성 추가, SecurityFilterChain 구성, PasswordEncoder 빈 등록, UserDetailsService 구현까지 인증 흐름의 기본 골격을 잘 구성했습니다. 리뷰/미션 조회의 페이징과 DTO 검증도 이전 주차 대비 구체화된 점이 좋습니다. 보완할 부분은 회원가입 입력 계약과 도메인 규칙의 경계, 중복 이메일 처리, Security 예외 타입과 권한 모델입니다. 특히 인증 기능은 Controller 허용 경로, UserDetailsService, PasswordEncoder, 사용자 저장 로직이 함께 맞물리므로 한 요청 흐름으로 테스트해 보는 것을 권장합니다.
|
|
||
| com.study.UMC10.domain.user.enums.Gender userGender; | ||
| try { | ||
| userGender = com.study.UMC10.domain.user.enums.Gender.valueOf(requestDto.gender().toUpperCase()); |
There was a problem hiding this comment.
gender가 null이거나 예상하지 못한 값이면 NONE으로 조용히 대체됩니다. 회원가입 입력값은 DTO 검증 또는 명시적인 도메인 예외로 실패시키는 편이 클라이언트 계약이 더 명확해집니다. Bean Validation과 Service 도메인 검증의 책임 경계를 함께 정리하는 것을 권장합니다.
| .status(com.study.UMC10.domain.user.enums.UserStatus.ACTIVE) | ||
| .build(); | ||
|
|
||
| User savedUser = userRepository.save(newUser); |
There was a problem hiding this comment.
저장 전에 이메일 중복 여부를 확인하는 흐름이 필요합니다. 같은 이메일이 여러 건 저장되면 UserDetailsService#findByEmail 인증 흐름도 모호해질 수 있으므로 Repository의 existsByEmail 또는 unique 제약과 도메인 예외를 함께 적용하는 것을 권장합니다.
| @Override | ||
| public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { | ||
| User user = userRepository.findByEmail(username) | ||
| .orElseThrow(() -> new UserException(UserErrorCode.MEMBER_NOT_FOUND)); |
There was a problem hiding this comment.
Spring Security의 DaoAuthenticationProvider 흐름에서는 사용자를 찾지 못한 경우 UsernameNotFoundException 계열로 전달되는 것이 자연스럽습니다. 도메인 예외를 그대로 던지면 인증 실패 처리와 공통 도메인 예외 처리의 경계가 흐려질 수 있으므로, Security 계층 예외와 API 도메인 예외를 분리하는 것을 권장합니다.
|
|
||
| @Override | ||
| public Collection<? extends GrantedAuthority> getAuthorities() { | ||
| return List.of(); |
There was a problem hiding this comment.
현재 권한 목록이 항상 비어 있어 이후 hasRole, hasAuthority 기반 인가 규칙을 추가하기 어렵습니다. 지금은 모든 인증 사용자에게 같은 권한을 주는 단계라도 ROLE_USER처럼 최소 권한을 명시해 두면 인증과 인가의 차이를 코드로 학습하기 좋습니다.
| ) | ||
|
|
||
| // 폼 로그인 설정 | ||
| .formLogin(form -> form |
There was a problem hiding this comment.
REST API 과제에서 formLogin을 사용할 경우 세션 기반 로그인, 로그인 페이지/리다이렉트, CSRF 설정이 함께 묶입니다. Swagger 테스트용 form login인지, 향후 JWT 기반 무상태 인증으로 전환할 것인지 인증 전략을 먼저 주석이나 설정 구조로 드러내는 것을 권장합니다.
|
|
||
| --- | ||
|
|
||
| # Stateful vs Stateless |
There was a problem hiding this comment.
Stateful/Stateless 비교가 잘 정리되어 있습니다. 여기에 현재 구현한 formLogin은 기본적으로 세션 기반 Stateful 인증에 가깝고, JWT는 Stateless 인증 흐름에 가깝다는 식으로 코드 설정과 연결하면 학습 내용의 적용력이 더 높아집니다.
2 participants
🔗 연관 이슈
🛠 작업 내용
🖼 스크린샷 (선택)
👀 리뷰 요구사항 (선택)
🤖 AI 활용
💬 나의 프롬프트
내가 작성한 코드에서 잘못 작성된 부분, 로직이 잘못된 부분이 있으면 해당 부분에 대해서 알려줘
🧠 AI 응답
잘못 작성된 로직에 대한 수정사항 + 오타 수정
✅ 내가 최종 선택한 방법 (이유)
💡 나만의 Tip (선택)