-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
3차 세미나 실습 과제 #13
base: main
Are you sure you want to change the base?
3차 세미나 실습 과제 #13
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
세빈님 과제하시느라 고생하셨습니다~
|
||
private final PostService postService; | ||
|
||
@PostMapping("/post") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest api에서는 단수형보다 복수형이 더 선호된다고 알고 있습니다!
그리고 글 작성시에 /api/v1/post로 보내시고 있는데
rest api�는 리소스의 식별이 url에 의해 명확하게 이루어 져야 한다 생각해서
저는 /v1/blogs/{blogId}/posts를 엔드포인트로 사용하고 있습니다
blogid를 pathvariable이 아닌 헤더로 보내주면 api가 어떤 블로그에 대해 글을 작성하는지 명확해지지 않는다고 생각하는데 세빈님의 생각은 어떠신지 궁금합니다
return ResponseEntity.status(HttpStatus.CREATED).header("Location", postService.writePost(blogId, memberId, postCreateRequest)) | ||
.body(SuccessStatusResponse.of(SuccessMessage.POST_CREATE_SUCCESS)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
status와 헤더를 직접 입력하는 대신에 .created를 사용해보아도 좋을 것 같아요
public void validateBlogMember(Long blogId, Long memberId) { | ||
Blog blog = this.findBlogById(blogId); | ||
if (!blog.getMember().getId().equals(memberId)) { | ||
throw new CustomizedException(ErrorMessage.UNAUTHORIZED_ACCESS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
�403대신 401을 사용하신 이유가 있으실까요?
만약 나중에 로그인이 제대로 구현되어 401에대한 처리는 필터쪽에서 공통적으로 처리해준다면
해당 비즈니스 로직에서는 블로그에 대한 작성 권한이 있는가에 대한 예외(403)를 발생시키는 것이 적합하다 생각합니다!
Blog blog = this.findBlogById(blogId); | ||
if (!blog.getMember().getId().equals(memberId)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
existsBy...를 사용해서 바로 boolean받아와도 좋을 것 같습니다!
public class CustomizedException extends BusinessException { | ||
public CustomizedException(ErrorMessage errorMessage) { | ||
super(errorMessage); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
비슷한 역할을 하는BusinessException이 있는데 CustomizedException을 만드신 이유가 있으실까요?
List<PostResponse> responses = posts.stream() | ||
.map(post -> new PostResponse(post.getId(), post.getTitle(), post.getContent())) | ||
.collect(Collectors.toList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dto로의 변환 작업은 service layer에서 수행하는 것이 어떨까요?
controller에서는 요청데이터의 유효성 검사, 서비스 메소드 호출, 응답상태, 데이터 전송정도만 하고
service에서 비즈니스로직, dto로의 변환을 해주는 것이 계층간의 역할분리에 도움이 된다 생각합니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
더해서 PostResponse에서 생성자가 아닌 정적 팩토리 메서드 패턴으로 매개변수로 Post 자체를 받으면
.map(PostResponse::of)
를 사용할 수 있는데, 한 번 고민해봐도 좋을 것 같아요 :)
} | ||
public static SuccessStatusResponse of(int status, String message, Object data) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위에 작성하신 것처럼 SuccessMessage로 캡슐화해서 넘겨주는 것은 어떨까요?
// 블로그 소유권 검증 | ||
blogService.validateBlogMember(blogId, memberId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 블로그 소유권에 대한 검증을 하신 이유를 알 수 있을까요?
블로그의 글은 블로그 주인 말고도 모두 접근할 수 있어야 된다 생각해서 여쭤봅니당
public ResponseEntity<SuccessStatusResponse> getPostById(@RequestHeader Long blogId, @RequestHeader Long memberId, | ||
@PathVariable Long postId) { | ||
Post post = postService.findPostById(blogId, memberId, postId); | ||
PostResponse postResponse = new PostResponse(post.getId(), post.getTitle(), post.getContent()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 마찬가지로 service에서 변환해주는 것이 좋을 것 같습니다.
// 블로그 소유권 검증 | ||
blogService.validateBlogMember(blogId, memberId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위와 같은 의문이 드네용
전체적으로 잘 구현하신 것 같네요! 고생하셨습니다!! |
public void validateBlogMember(Long blogId, Long memberId) { | ||
Blog blog = this.findBlogById(blogId); | ||
if (!blog.getMember().getId().equals(memberId)) { | ||
throw new CustomizedException(ErrorMessage.UNAUTHORIZED_ACCESS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 알기론 권한에 대한 문제는 403에러가 맞는 것 같습니다! 혹시라도 401에러를 반환해주신 특별한 이유가 있으신가요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
세빈님 고생하셨습니당~!
덕분에 몰랐던 어노테이션 하나 알아가요
3주동안 코드리뷰조 같이 해서 좋았어요 ㅎㅁㅎ
import jakarta.validation.constraints.Size; | ||
|
||
public record PostCreateRequest( | ||
@NotBlank(message = "제목을 입력해주세요.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@notblank 어노테이션은 처음 보는데 입력이 공백으로 처리되는 걸 막아주는 건가요? 궁금해서 남깁니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
과제하느라 고생하셨습니다 :)
몇가지 좋은 코멘트가 이미 많이 담겨 있어서 구경헀네요!
저도 몇가지 남겨놓았으니 코드리뷰에 대한 답글 달아주세요 >< 모두들 궁금해하고 있네요!
화이팅입니다!
List<PostResponse> responses = posts.stream() | ||
.map(post -> new PostResponse(post.getId(), post.getTitle(), post.getContent())) | ||
.collect(Collectors.toList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
더해서 PostResponse에서 생성자가 아닌 정적 팩토리 메서드 패턴으로 매개변수로 Post 자체를 받으면
.map(PostResponse::of)
를 사용할 수 있는데, 한 번 고민해봐도 좋을 것 같아요 :)
return ResponseEntity.ok(new SuccessStatusResponse( | ||
SuccessMessage.POST_FIND_SUCCESS.getStatus(), | ||
SuccessMessage.POST_FIND_SUCCESS.getMessage(), | ||
postResponse | ||
)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Response에도 정적 팩터리 메서드 패턴을 사용해도 좋을 것 같아요!
private final BlogService blogService; | ||
private final MemberService memberService; | ||
|
||
public String writePost(Long blogId, Long memberId, PostCreateRequest postCreateRequest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
save를 수행할 경우 @Transactional
을 사용하지 않아도 됩니다!!!
🍃 Issue
close #12
⚡️ 구현 내용
🔑 api 명세서(클릭)
① Exception
CustomizedException.java
에러메세지를 반환하기 위해서 구현
SuccessStatusResponse.java
GET API 에서 SuccessStatusResponse를 Data를 담아보낼 수 있도록 변경
② Blog
멤버의 블로그 소유권을 검증하는 절차가 한 번만 나오는 것이 아닌, 여러 메소드에서 검증하는 것을 보고 이를 따로 메서드로 분리해서 검증하고자 구현한 메서드
만약 둘이 불일치 할 시 UNAUTHORIZED_ACCESS 메세지를 반환합니다.
③ Post
POST - 블로그 글 작성
PostCreateRequest.java
블로그 글 작성 시 해당 조건을 지켜야 작성할 수 있도록 Validation을 설정해주었습니다.
PostService.java
PostController.java
@Valid 지정
memberId의 blogId와 header의 blogId 불일치
blogId의 memberId와 header의 memberId 불일치
GET - 작성된 글 전체 불러오기
PostResponse.java
PostRepository.java
PostService.java
PostController.java
GET - 작성된 글 한개 불러오기
PostService.java
PostController.java
🍀 결과
POST - 블로그 글 작성
GET - 작성된 글 전체 불러오기
GET - 작성된 글 한개 불러오기
?의문점 및 고려사항