[Feature] 유저 프로필 비즈니스 로직, 포스트 조회 로직 구현 #61
Conversation
Summary of ChangesHello @hyobin-yang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 사용자 프로필 관리 기능을 전면적으로 개선하고, 유저별 게시물 조회 기능을 확장하여 애플리케이션의 핵심 사용자 경험을 향상시키는 데 중점을 둡니다. 유저가 자신의 정보를 보다 세밀하게 관리하고, 작성하거나 스크랩한 콘텐츠를 쉽게 탐색할 수 있도록 새로운 비즈니스 로직, API 엔드포인트, 데이터베이스 스키마 변경 및 관련 테스트를 포함합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 유저 프로필 관련 비즈니스 로직과 포스트 조회 로직을 구현하는 것을 목표로 하고 있습니다. 전반적으로 기능 구현이 잘 이루어졌으며, 특히 회원 탈퇴 처리, 비활성 유저 로그인 방지, 사용자별 포스트 조회 기능 추가 등 중요한 기능들이 포함되었습니다. 테스트 코드도 충실하게 작성되어 코드의 안정성을 높이는 데 기여했습니다. 다만, 몇 가지 개선점을 제안드립니다. 보안적으로 중요한 권한 검사 로직이 누락된 부분이 있어 수정이 필요하며, 코드 중복을 줄이고 유지보수성을 높이기 위한 리팩토링 제안, 그리고 API 설계 관점에서의 개선 사항 등을 포함했습니다. 자세한 내용은 각 파일의 리뷰 코멘트를 참고해주세요.
| @GetMapping("/{userId}/drafts") | ||
| @ResponseStatus(HttpStatus.OK) | ||
| public PageResponseDto<PostResponseDto> getUserDraftPosts( | ||
| @PathVariable Long userId, | ||
| @ModelAttribute PageRequestDto request, | ||
| User user | ||
| ){ | ||
| return postQueryService.getUserDraftPosts(userId, request); | ||
| } |
There was a problem hiding this comment.
보안 취약점: 다른 사용자의 임시저장 게시글을 조회할 수 있습니다. 현재 구현에서는 userId만으로 임시저장 글을 조회하여, 인증된 사용자가 다른 사용자의 userId를 알면 해당 유저의 비공개 임시저장 글을 볼 수 있습니다. User user 파라미터는 현재 인증된 사용자를 나타내므로, 요청된 userId가 인증된 사용자의 ID와 일치하는지 확인하는 로직이 반드시 추가되어야 합니다. 또한, user 파라미터가 현재 사용되지 않고 있습니다.
| @GetMapping("/{userId}/drafts") | |
| @ResponseStatus(HttpStatus.OK) | |
| public PageResponseDto<PostResponseDto> getUserDraftPosts( | |
| @PathVariable Long userId, | |
| @ModelAttribute PageRequestDto request, | |
| User user | |
| ){ | |
| return postQueryService.getUserDraftPosts(userId, request); | |
| } | |
| public PageResponseDto<PostResponseDto> getUserDraftPosts( | |
| @PathVariable Long userId, | |
| @ModelAttribute PageRequestDto request, | |
| User user | |
| ){ | |
| if (user == null || !user.getId().equals(userId)) { | |
| throw new BusinessException("자신의 임시저장 글만 조회할 수 있습니다."); | |
| } | |
| return postQueryService.getUserDraftPosts(userId, request); | |
| } | |
| @GetMapping("/{userId}/scraps") | ||
| @ResponseStatus(HttpStatus.OK) | ||
| public PageResponseDto<PostResponseDto> getUserScrappedPosts( | ||
| @PathVariable Long userId, | ||
| @ModelAttribute PageRequestDto request, | ||
| User user | ||
| ){ | ||
| return postQueryService.getUserScrappedPosts(userId, request); | ||
| } |
There was a problem hiding this comment.
보안 취약점: 다른 사용자가 스크랩한 게시글 목록을 조회할 수 있습니다. 임시저장 글과 마찬가지로, 스크랩한 게시글 목록은 보통 비공개 정보입니다. 현재 구현은 userId만으로 스크랩 목록을 조회하므로, 다른 사용자의 스크랩 목록을 무단으로 열람할 수 있는 심각한 보안 문제가 있습니다. 인증된 사용자(user)의 ID와 요청된 userId가 일치하는지 확인하는 권한 검사 로직을 추가해야 합니다.
public PageResponseDto<PostResponseDto> getUserScrappedPosts(
@PathVariable Long userId,
@ModelAttribute PageRequestDto request,
User user
){
if (user == null || !user.getId().equals(userId)) {
throw new BusinessException("자신이 스크랩한 글만 조회할 수 있습니다.");
}
return postQueryService.getUserScrappedPosts(userId, request);
}| private void sendForEmailChange(EmailVerificationRequestDto request) { | ||
| if (userRepository.existsByEmail(request.getEmail()) | ||
| && !request.getEmail().equals(request.getOriginalEmail())) { | ||
| throw new BusinessException("이미 가입되어 있는 이메일입니다."); | ||
| } | ||
| sendVerificationCode(request); | ||
| } |
There was a problem hiding this comment.
이메일 변경을 위한 인증 메일 발송 시 originalEmail이 null인 경우에 대한 처리가 없습니다. EmailVerificationRequestDto에서 originalEmail이 nullable(String?)로 선언되어 있어 클라이언트에서 이 값을 보내지 않을 수 있습니다. 이 경우 request.getOriginalEmail()은 null이 되고, !request.getEmail().equals(null)은 항상 true가 되어, 사용자가 자신의 현재 이메일을 재인증하려는 경우에도 '이미 가입되어 있는 이메일'이라는 잘못된 오류가 발생할 수 있습니다. EMAIL_CHANGE 목적일 때는 originalEmail이 필수적으로 존재해야 하므로, null 체크 로직을 추가하는 것이 안전합니다.
private void sendForEmailChange(EmailVerificationRequestDto request) {
if (request.getOriginalEmail() == null) {
throw new BusinessException("이메일 변경 시에는 기존 이메일 정보가 필요합니다.");
}
if (userRepository.existsByEmail(request.getEmail())
&& !request.getEmail().equals(request.getOriginalEmail())) {
throw new BusinessException("이미 가입되어 있는 이메일입니다.");
}
sendVerificationCode(request);
}| @Transactional | ||
| public void updateUserProfile(User user, UserProfileUpdateRequestDto request){ | ||
| UpdateVo vo = new UpdateVo( | ||
| request.getProfileImageUrl(), | ||
| request.getNickname(), | ||
| request.getBio() | ||
| ); | ||
| user.update(vo); | ||
| } |
There was a problem hiding this comment.
프로필 업데이트 시 닉네임 중복 확인 로직이 누락되었습니다. User 엔티티의 nickname 필드에는 unique 제약조건이 설정되어 있어, 중복된 닉네임으로 변경 시 데이터베이스 레벨에서 오류가 발생합니다. 이는 사용자에게 친절하지 않은 500 에러를 반환할 수 있습니다. 프로필 업데이트 전에 변경하려는 닉네임이 현재 사용자의 닉네임과 다르고, 다른 사용자가 이미 사용 중인지 확인하는 로직을 추가해야 합니다.
@Transactional
public void updateUserProfile(User user, UserProfileUpdateRequestDto request){
if (!user.getNickname().equals(request.getNickname()) && userRepository.existsByNickname(request.getNickname())) {
throw new BusinessException("이미 사용중인 닉네임입니다.");
}
UpdateVo vo = new UpdateVo(
request.getProfileImageUrl(),
request.getNickname(),
request.getBio()
);
user.update(vo);
}| @GetMapping("/verify-user-password") | ||
| public ResponseEntity<Map<String, Boolean>> verifyUserPassword( | ||
| @RequestBody @Valid PasswordRequestDto request, User user) { | ||
| boolean isValid = userService.verifyUserPassword(user, request); | ||
| return ResponseEntity.ok(Map.of("유저 비밀번호 일치 여부 ", isValid)); | ||
| } |
There was a problem hiding this comment.
API 설계: GET 요청에 @RequestBody를 사용하는 것은 HTTP 명세에 어긋나지는 않지만, 많은 클라이언트와 프레임워크에서 지원하지 않는 비표준적인 방식입니다. 비밀번호와 같은 민감한 정보를 전송하므로 POST 메서드를 사용하는 것이 더 적절하고 안전합니다. GET 메서드는 보통 멱등성을 가지며 리소스를 조회하는 데 사용됩니다.
| @GetMapping("/verify-user-password") | |
| public ResponseEntity<Map<String, Boolean>> verifyUserPassword( | |
| @RequestBody @Valid PasswordRequestDto request, User user) { | |
| boolean isValid = userService.verifyUserPassword(user, request); | |
| return ResponseEntity.ok(Map.of("유저 비밀번호 일치 여부 ", isValid)); | |
| } | |
| @PostMapping("/verify-user-password") | |
| public ResponseEntity<Map<String, Boolean>> verifyUserPassword( | |
| @RequestBody @Valid PasswordRequestDto request, User user) { | |
| boolean isValid = userService.verifyUserPassword(user, request); | |
| return ResponseEntity.ok(Map.of("유저 비밀번호 일치 여부 ", isValid)); | |
| } | |
| public PageResponseDto<PostResponseDto> getUserPublishedPosts(Long userId, PageRequestDto pageRequest){ | ||
| List<Post> posts = postQueryRepository.getUserPublishedPostsWithPaging(userId, pageRequest); | ||
|
|
||
| return pagingUtils.createPageResponse( | ||
| posts, | ||
| pageRequest.getValidatedSize(), | ||
| PostResponseDto::from, | ||
| Post::getCreatedAt, | ||
| Post::getId | ||
| ); | ||
| } | ||
|
|
||
| public PageResponseDto<PostResponseDto> getUserDraftPosts(Long userId, PageRequestDto pageRequest){ | ||
| List<Post> posts = postQueryRepository.getUserDraftPostsWithPaging(userId, pageRequest); | ||
|
|
||
| return pagingUtils.createPageResponse( | ||
| posts, | ||
| pageRequest.getValidatedSize(), | ||
| PostResponseDto::from, | ||
| Post::getCreatedAt, | ||
| Post::getId | ||
| ); | ||
| } | ||
|
|
||
| public PageResponseDto<PostResponseDto> getUserScrappedPosts(Long userId, PageRequestDto pageRequest){ | ||
| List<Post> posts = postQueryRepository.getUserScrappedPostsWithPaging(userId, pageRequest); | ||
|
|
||
| return pagingUtils.createPageResponse( | ||
| posts, | ||
| pageRequest.getValidatedSize(), | ||
| PostResponseDto::from, | ||
| Post::getCreatedAt, | ||
| Post::getId | ||
| ); | ||
| } |
There was a problem hiding this comment.
getUserPublishedPosts, getUserDraftPosts, getUserScrappedPosts 세 메서드에서 pagingUtils.createPageResponse를 호출하는 로직이 중복됩니다. 이 부분을 별도의 private 메서드로 추출하여 코드 중복을 줄이고 가독성을 높일 수 있습니다.
public PageResponseDto<PostResponseDto> getUserPublishedPosts(Long userId, PageRequestDto pageRequest){
List<Post> posts = postQueryRepository.getUserPublishedPostsWithPaging(userId, pageRequest);
return createPostPageResponse(posts, pageRequest);
}
public PageResponseDto<PostResponseDto> getUserDraftPosts(Long userId, PageRequestDto pageRequest){
List<Post> posts = postQueryRepository.getUserDraftPostsWithPaging(userId, pageRequest);
return createPostPageResponse(posts, pageRequest);
}
public PageResponseDto<PostResponseDto> getUserScrappedPosts(Long userId, PageRequestDto pageRequest){
List<Post> posts = postQueryRepository.getUserScrappedPostsWithPaging(userId, pageRequest);
return createPostPageResponse(posts, pageRequest);
}
private PageResponseDto<PostResponseDto> createPostPageResponse(List<Post> posts, PageRequestDto pageRequest) {
return pagingUtils.createPageResponse(
posts,
pageRequest.getValidatedSize(),
PostResponseDto::from,
Post::getCreatedAt,
Post::getId
);
}| private static PostType getPostType(Post post) { | ||
| if (post instanceof StoryPost) { | ||
| return PostType.STORY; | ||
| } else if (post instanceof FreePost) { | ||
| return PostType.FREE; | ||
| } else if (post instanceof CurationPost) { | ||
| return PostType.CURATION; | ||
| } | ||
| throw new BusinessException("유효하지 않은 Post 타입입니다: " + post.getClass().getName()); | ||
| } |
There was a problem hiding this comment.
getPostType 메서드에서 instanceof를 사용하여 Post 타입을 확인하는 방식은 새로운 Post 타입이 추가될 때마다 이 DTO를 수정해야 하는 단점이 있습니다. 이는 OCP(개방-폐쇄 원칙)에 위배될 수 있습니다. 보다 객체지향적인 설계를 위해 Post 추상 클래스에 getPostType() 추상 메서드를 선언하고, 각 하위 클래스(StoryPost, FreePost 등)에서 이를 구현하여 자신의 타입을 반환하도록 리팩토링하는 것을 고려해 보세요. 이렇게 하면 PostResponseDto는 Post의 구체적인 하위 타입에 대해 알 필요가 없어져 유지보수성이 향상됩니다.
| @Override | ||
| public List<Post> getUserPublishedPostsWithPaging(Long userId, PageRequestDto pageRequest) { | ||
| JPAQuery<Post> query = queryFactory | ||
| .selectFrom(post) | ||
| .leftJoin(post.user, user).fetchJoin() | ||
| .where( | ||
| post.user.id.eq(userId) | ||
| .and(post.postStatus.eq(PostStatus.PUBLISHED)) | ||
| .and(post.isBlocked.isFalse()) | ||
| ); | ||
|
|
||
| return pagingUtils.applyCursorPagination( | ||
| query, | ||
| pageRequest, | ||
| post.createdAt, | ||
| post.id | ||
| ); | ||
| } | ||
|
|
||
| @Override | ||
| public List<Post> getUserDraftPostsWithPaging(Long userId, PageRequestDto pageRequest) { | ||
| JPAQuery<Post> query = queryFactory | ||
| .selectFrom(post) | ||
| .leftJoin(post.user, user).fetchJoin() | ||
| .where( | ||
| post.user.id.eq(userId) | ||
| .and(post.postStatus.eq(PostStatus.DRAFT)) | ||
| .and(post.isBlocked.isFalse()) | ||
| ); | ||
|
|
||
| return pagingUtils.applyCursorPagination( | ||
| query, | ||
| pageRequest, | ||
| post.createdAt, | ||
| post.id | ||
| ); | ||
| } |
There was a problem hiding this comment.
getUserPublishedPostsWithPaging와 getUserDraftPostsWithPaging 메서드의 구조가 PostStatus만 다르고 거의 동일합니다. PostStatus를 파라미터로 받는 private 헬퍼 메서드를 만들어 코드 중복을 제거할 수 있습니다. 이렇게 하면 유지보수성이 향상됩니다.
@Override
public List<Post> getUserPublishedPostsWithPaging(Long userId, PageRequestDto pageRequest) {
return getUserPostsByStatusWithPaging(userId, pageRequest, PostStatus.PUBLISHED);
}
@Override
public List<Post> getUserDraftPostsWithPaging(Long userId, PageRequestDto pageRequest) {
return getUserPostsByStatusWithPaging(userId, pageRequest, PostStatus.DRAFT);
}
private List<Post> getUserPostsByStatusWithPaging(Long userId, PageRequestDto pageRequest, PostStatus status) {
JPAQuery<Post> query = queryFactory
.selectFrom(post)
.leftJoin(post.user, user).fetchJoin()
.where(
post.user.id.eq(userId)
.and(post.postStatus.eq(status))
.and(post.isBlocked.isFalse())
);
return pagingUtils.applyCursorPagination(
query,
pageRequest,
post.createdAt,
post.id
);
}| data class PasswordRequestDto( | ||
| @get:Pattern( | ||
| regexp = "^(?=.*[a-z])(?=.*[A-Z])(?=.*[0-9])(?=.*[!@#\$%^&*()]).{10,}\$", | ||
| message = "비밀번호는 영어 대문자와 소문자, 숫자, 특수문자를 모두 포함하여 10자 이상이어야 합니다" | ||
| ) | ||
| val password: String | ||
| ) No newline at end of file |
There was a problem hiding this comment.
com.daramg.server.auth.dto 패키지에도 동일한 이름의 PasswordRequestDto 클래스가 존재합니다. 두 클래스는 필드 구성이 달라 서로 다른 목적으로 사용되는데, 이름이 같아 혼동을 유발할 수 있습니다. 예를 들어, auth 패키지의 DTO는 PasswordResetRequestDto로, user 패키지의 이 DTO는 PasswordChangeRequestDto 또는 UserPasswordDto 등으로 역할을 명확히 나타내는 이름으로 변경하는 것을 권장합니다. 이는 코드의 가독성과 유지보수성을 높여줍니다.
| @get:Size(max = 12, message = "bio는 12자 이하로 입력해주세요") | ||
| val bio: String? |
There was a problem hiding this comment.
bio 필드의 최대 길이가 @Size(max = 12)로 설정되어 있습니다. 12자는 자기소개를 입력하기에 너무 짧아 보입니다. User 엔티티에서 bio 컬럼은 TEXT 타입으로 정의되어 있어 더 긴 내용을 저장할 수 있습니다. 사용자 경험을 위해 이 길이를 더 늘리는 것을 고려해 보세요. 예를 들어 120자 정도로 늘리는 것이 어떨까요?
No description provided.