Open limseulki opened 1 year ago
return ErrorResponse.toResponseEntityValid(sb.toString(), HttpStatus.BAD_REQUEST);
ResponseEntity<ErrorResponse> toResponseEntityValid(String errorCode, HttpStatus httpStatus)
가 항상 유효성 검사에만 사용하고, 항상 BadRequest라면 HttpStatus를 매개변수로 받지 않아도 될 것 같아요!
@PostMapping("")
는 /를 생략하지 않고 정확하게 써주는게 좋을 것 같아요~@DeleteMapping("/{id}")
public Message deleteComment(@PathVariable Long id, @AuthenticationPrincipal UserDetailsImpl userDetails) {
return commentService.deleteComment(id, userDetails.getUser());
}
모든 메서드에 반드시 response가 있을 필요는 없습니다. 정상적으로 끝났는지는 HttpStatus로도 알 수 있기 때문에 '성공 메세지'가 '에러 메세지'의 경우와 비교해서 생각해보면 좋을 것 같아요. 에러메세지의 경우 클라이언트에서 alert 등으로 띄워주기도 합니다. 프로젝트 주차에 프론트엔드 개발자와 api 명세서를 작성하면서 맞춰보세요.
@Column(nullable = false)
private int commentLike;
Comment가 이미 Like와 연관관계 매핑이 되어 있기 때문에 commentLike 필드가 필요하지 않을 것 같아요. 연관관계를 충분히 활용해보세요.
public class Like {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "user_id") private User user;
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "post_id") private Post post;
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "comment_id") private Comment comment;
현재 Like가 Post와 Comment를 모두 가지고 있는데, 다른 방법도 있을 것 같아요. Post 좋아요와 Comment 좋아요는 완전히 다른 기능이기 때문에 현재 Like는 user, post, comment가 모두 값을 가지고 있는 경우는 절대 없고 반드시 하나의 값은 null이 됩니다. 정말 nullable한 경우라면 괜찮겠지만, 이 경우에는 엔티티 연관관계 상 null이 되는 거라서 다른 방법을 생각해보면 좋을 것 같아요. post 좋아요와 comment 좋아요 값만 필요한 경우에도 이 상태에서는 반드시 select 조건이 필요해지구요.
if (like == null) {
likeRepository.save(new Like(user, post, null));
post.like();
return new Message("게시글 좋아요 성공", 200);
} else { // 좋아요 있으면 DB에서 제거
likeRepository.deleteById(like.getId());
post.unlike();
return new Message("게시글 좋아요 취소 성공", 200);
}
좋아요와 좋아요 삭제가 모두 동일한 api로 구현이 되어 있는데, HttpMethod를 고려하면 POST와 DELETE는 분명히 다른 것 같아요.
좋아요의 url이 @RequestMapping("/api/like") + @PostMapping("/post/{postId}") 인데, restApi 설계규칙의 계층관계와 연관관계를 고려해서 수정해보세요. 관련해서 LikeController, LikeService도 함께 생각해보면 좋을 것 같네요!
@PutMapping("/post/{id}")
restApi 설계규칙의 단/복수에 대해서도 찾아보세요!
private List<CommentResponseDto> getCommentList(Long postId) {
// 게시글에 달린 댓글 찾아서 작성일 기준 내림차순 정렬
List<Comment> commentList = commentRepository.findAllByPostIdOrderByCreatedAtDesc(postId);
List<CommentResponseDto> commentResponseDtoList = new ArrayList<>();
for(Comment comment : commentList) {
commentResponseDtoList.add(new CommentResponseDto(comment));
}
return commentResponseDtoList;
}
전체적으로 양방향으로 맵핑되어있는데 Service 로직들은 사실상 양방향 맵핑을 사용하고 있지 않은 것 같아요. 양방향으로 맵핑하신 이유가 있는지 궁금합니다!
현재까지 commit된 내용들은 lv4, 5에 기능을 조금 추가하고, 메소드 분리를 한 것인데, 전체적인 코드 리뷰를 받아보고 싶습니다.
추가된 기능은 아래와 같습니다.
구두로 피드백주신 내용 중, 공통 응답 부분과 유효성 검증 서비스단으로 옮겨보는 것은 추가로 해보려고 합니다. 페이징/정렬은 페어로 구현했고, 제 코드에도 추가 반영 예정입니다.