Refactor : 반례리스트 조회 - #406
Conversation
Test Results 27 files 27 suites 6s ⏱️ Results for commit 90c4eb6. ♻️ This comment has been updated with latest results. |
|
저희 비회원인 경우도 좋아요를 누를 수 있는건가요? |
그걸 막은거에요 !.!.! |
| if (jwt != null) | ||
| return userRepository.findByEmail(tokenProvider.getUserEmail(jwt)) | ||
| .orElseThrow(() -> new UserValidationException("없는 사용자 입니다.")); | ||
| else |
There was a problem hiding this comment.
AuthedUser어노테이션 기존 코드랑 변경한 코드가 무슨 차이가 있는거에요?
There was a problem hiding this comment.
기존에는 로그인하지 않아 user객체에 정보가 없을 땐 예외를 바로 발생했는데, 지금 바꾼건 AuthUser(required = false) 라면 예외를 발생시키지 않고 통과하도록 바꾼거에요!
There was a problem hiding this comment.
필요하다고 해서요..! 유저가 아닌 경우에도 반례게시판을 봐야한다고 해서 진행했습니다
There was a problem hiding this comment.
비회원인 경우와 회원인 경우를 모두 처리해야해서, AuthUser 어노테이션 부분을 수정했습니다.
--> 이 부분때문에요
rladmstn
left a comment
There was a problem hiding this comment.
수고하셨어요 👍
코멘트 확인 부탁드립니다!
| public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, | ||
| NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { | ||
| AuthedUser authedUser = parameter.getParameterAnnotation(AuthedUser.class); | ||
| boolean required = (authedUser == null) || authedUser.required(); |
There was a problem hiding this comment.
이거 authedUser == null이면 왜 required가 true인지 잘 이해가 안가요..!
제가 이해하기로는 Controller에 @AuthedUser 애노테이션이 없는 케이스인 것 같은데, 그럼 인증이 필요 없는 API인거 아닐까요??
There was a problem hiding this comment.
회원인 경우와 비회원인 경우에 따라 따로따로 처리해야하는 로직이라서 @authuser이 필요해서 바꾼거에요!
There was a problem hiding this comment.
비회원인 경우에 반례게시판을 볼 수 있지만, 좋아요는 누를 수 없도록 처리해야해서 이렇게 진행했습니다
There was a problem hiding this comment.
지금 반례리스트 조회할 때 비회원도 가능해서 @AuthedUser(required=false)인건 이해됩니다.
근데 그럼 EdgeCaseLike에서 좋아요를 누를 수 있도록 하게 하려면, 좋아요 추가 API의 컨트롤러에 @AuthedUser(required=true)로 추가해야 하는거 아닐까요??
지금 로직에서는 컨트롤러에서 @AuthedUser를 쓰지 않으면 인증이 필요하다고 판단하게 되지 않나요??
There was a problem hiding this comment.
@AuthedUser를 쓰지 않으면 인증이 필요하다고 판단하게 되지 않나요?? 맞아요 ! 좋아요 추가 API의 컨트롤러에 @AuthedUser()로 이미 존재하지 않나요? default값이 true라서 괜찮을거같다고 생각했습니다..!
There was a problem hiding this comment.
boolean required = (authedUser == null) || authedUser.required();
위 조건에서 authedUser == null 이 이해가 안된거였습니다.
authedUser == null은 컨트롤러에 아예 @AuthedUser 애노테이션이 없는 경우인 것 같은데 왜 인증 정보 required가 true이도록 조건을 걸어야 하는지요!
저는@AuthedUser 애노테이션 자체가 없으면 인증 정보는 필요없는 API라는게 더 자연스러운 문맥이지 않을까 싶었거든요.
There was a problem hiding this comment.
아!! 이해했습니다. 저 authedUser == null부분을 지우는게 더 맞는거같네요..!
There was a problem hiding this comment.
아!! 코멘트 정정합니다!
해당 resolveArgument() 메서드 내에서는 authedUser가 null일 경우가 없어서 authedUser == null 조건이 필요 없는겁니다!
애초에 @AuthedUser가 안쓰였으면 supportsParameter()에서 false가 나와버리고, 이 resolver도 사용이 안되겠군요!
| if (user != null && !edgeCaseList.isEmpty()) { | ||
| List<EdgeCaseLike> myLikes = | ||
| edgeCaseLikeRepository.findByUserAndEdgeCaseIn(user, edgeCaseList); | ||
|
|
||
| likedEdgeCaseIds = myLikes.stream() | ||
| .map(like -> like.getEdgeCase().getId()) | ||
| .collect(Collectors.toSet()); | ||
| } |
There was a problem hiding this comment.
QueryDSL 사용해서 동적 쿼리로 DTO를 만들 수는 있을 것 같은데 많이 복잡할 것 같네요!
그래도 반례 하나마다 쿼리가 나가는 것보단 지금이 더 나은 것 같긴 합니다!
| public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, | ||
| NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { | ||
| AuthedUser authedUser = parameter.getParameterAnnotation(AuthedUser.class); | ||
| boolean required = (authedUser == null) || authedUser.required(); |
There was a problem hiding this comment.
아!! 코멘트 정정합니다!
해당 resolveArgument() 메서드 내에서는 authedUser가 null일 경우가 없어서 authedUser == null 조건이 필요 없는겁니다!
애초에 @AuthedUser가 안쓰였으면 supportsParameter()에서 false가 나와버리고, 이 resolver도 사용이 안되겠군요!
Co-authored-by: Willie <150939763+junggyo1020@users.noreply.github.com> Co-authored-by: shshwn <87060472+s-hwan@users.noreply.github.com>
📌 Related Issue
be-48
🚀 Description
📢 Review Point
📚Etc (선택)