-
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
Seminar/3 #4
base: main
Are you sure you want to change the base?
Seminar/3 #4
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.
은서님 수고많으셨습니다!!!
저번에 한 번 도와드려서 그런지 리뷰하기가 수월했네요!!
다만 세미나 과제 누락된 부분이 조금 있어보입니다! 편하게 질문하시면서 고쳐 나가봅시다!
합세까지 1주일 남았으니 이번 세미나 과제 마무리에 조금 더 힘써주시면 좋을 거 같아요!
@PostMapping("/blog") | ||
public ResponseEntity<SuccessStatusResponse> createBlog( | ||
@RequestHeader(name = "memberId") Long memberId, | ||
@RequestBody BlogCreateRequest blogCreateRequest |
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 class PostController { | ||
|
||
private final PostService postService; | ||
@PostMapping("/posts") |
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.
현재 위 blog와 member controller에서는 rest api의 url을 단수형 (member, blog)로 해주신 상황인데 여기에는 왜 posts인 복수형을 사용하셨는지 궁금합니다! 아래 블로그 참고하셔서 은서님만의 컨벤션을 정해주시면 좋을 거 같아요!!
@PostMapping("/posts") | ||
public ResponseEntity<SuccessStatusResponse> createPost( | ||
@RequestHeader Long blogId, | ||
@RequestBody 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.
마찬가지로 검증해주는 과정 필요합니다!
this.age = age; | ||
} | ||
|
||
public static Member create(String name, Part part, int age) { |
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.
Blog에서는 빌더를 사용하지 않고 Member에서는 빌더를 사용한 이유가 있을까요??
만약 세미나 코드를 따라치는 과정에서 크게 신경을 쓰지 못하셨다면 다음 세미나인 합세 전까지는 본인의 컨벤션을 정하시는게 좋아보여요~!
private final MemberService memberService; | ||
|
||
|
||
public String create(Long memberId, BlogCreateRequest blogCreateRequest) { |
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.
create에는 transactional을 적어주지 않은 이유가 있을까요??
저는 개인적으로 Service 위에 Transactional(readOnly = true)를 달아주고 트랜잭션이 필요한 메소드 위에 트랜잭션 어노테이션을 달아주는 편입니다.
jparepository의 save에 transaction 어노테이션이 달려있어서 다행히 코드는 문제 없이 동작하지만 이러한 점을 인지하시고 코드를 짜시는 거랑 모르시는 상태로 코드를 짜는 건 큰 차이가 있습니다!
세미나 코드 한 줄 한 줄 정말 배울 점이 많으니 한 번 천천히 뜯어보시면서 주변 오비들한테 계속해서 질문하시고 공부하시면 좋을 것 같아요!
|
||
private final PostRepository postRepository; | ||
private final BlogService blogService; | ||
public String create(Long blogId, 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.
위 리뷰와 마찬가지로 트랜잭션 어노테이션에 대해 고민해보시면 좋을 거 같아요!
그리고 현재 코드에서 멤버의 블로그인지 검증하는 부분이 누락되어있습니다!
은서님의 블로그인데 제가 글을 쓰면 안되겠죠?? 검증하는 로직도 한 번 달아봅시다!!!
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class PostService { |
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.
게시글 조회 메소드 누락되어있습니다!
@@ -0,0 +1,7 @@ | |||
package org.sopt.demo.service.dto; | |||
|
|||
public record BlogCreateRequest( |
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.
검증하는 부분을 요기에 추가해주세요!
|
||
import org.sopt.demo.domain.Part; | ||
|
||
public record MemberCreateDto( |
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.
여기에도 검증~!
@@ -0,0 +1,7 @@ | |||
package org.sopt.demo.service.dto; | |||
|
|||
public record 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.
검증 로직~!!!
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.
과제하시느라 수고하셨습니다~!!!
@PostMapping("/posts") | ||
public ResponseEntity<SuccessStatusResponse> createPost( | ||
@RequestHeader Long blogId, | ||
@RequestBody 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.
궁찬님께서 이미 언급해주셨지만, Post 되는 객체에 대해 Request body validation이 누락되어 있는거 같아요!
파라미터로 @RequestBody 어노테이션 옆에 @Valid를 작성하면, RequestBody로 들어오는 객체에 대한 검증을 수행할 수 있습니다:)
public static Post create(Blog blog, String title, String content) { | ||
Post post = new Post(); | ||
post.blog = blog; | ||
post.title = title; | ||
post.content = content; | ||
return 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.
위의 코드는 아래와 같이 간단하게 줄여서 쓸 수 있습니다.
public static Post create(Blog blog, String title, String content) {
return new Post(title, content, blog);
}
현재 은서님은 create 메소드가 기본생성자를 호출한 후 필드들을 초기화하도록 코드를 구현하셨는데, 이미 매개변수가 있는 생성자를 정의해두었기 때문에 이 생성자를 활용하여 객체 생성과 필드 초기화를 동시에 처리하는 것이 더 합리적일 것 같아요.
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 interface PostRepository extends JpaRepository<Post, Long> { | ||
|
||
|
||
} |
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.
저 또한 코드 리뷰를 통해서 공통적으로 지적받은 부분인데요,, 코드의 공백라인 부분에서 일관성이 없는 부분이 보입니다. 가령 바로 위의 코드인 MemberRepository
와 비교해보았을 때, 현재 코드에서는 불필요한 공백 라인이 눈에 띕니다!
인텔레제이에서는 맥북의 경우, Command + Option + L 단축키를 사용하시면 코드라인 자동 정렬이 가능하다고 합니다:D
public String create(Long blogId, PostCreateRequest postCreateRequest) { | ||
Blog blog = blogService.findById(blogId); | ||
Post post = new Post(postCreateRequest.title(), postCreateRequest.content(),blog); |
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.
Post 객체를 만들기 전에, 블로그를 소유하지 않은 사용자가 해당 블로그에 글을 작성할 경우 발생할 예외처리 진행 부분을 누락하신 것 같아요.
저의 경우, create 함수 매개변수로 memberId
를 추가적으로 받아오고, 그 아이디가 blog.getMember()
를 통해 얻어온 아이디의 일치 여부를 통해서 구현하였습니다:)
import jakarta.validation.constraints.Size; | ||
|
||
public record BlogTitleUpdateRequest( | ||
@Size(max = 10, message = "블로그 제목이 최대 글자 수(10자)를 초과했습니다.") String title |
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.
블로그 제목이 Null 값이 되는 것을 방지하기 위해 @NotNull 어노테이션을 추가하시는 것도 좋아보여요.
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.
NotNull을 사용한다면 "", 나 " " 같은 경우는 통과가 됩니다.
NotBlank와 NotEmpty에 대해 공부해보셔도 좋을 거 같습니다~!
public MemberFindDto findMemberById(Long memberId) { | ||
return MemberFindDto.of(memberRepository.findById(memberId).orElseThrow( | ||
() -> new EntityNotFoundException("ID에 해당하는 사용자가 존재하지 않습니다.") | ||
)); | ||
} | ||
|
||
@Transactional | ||
public void deleteMemberById(Long memberId) { | ||
Member member = memberRepository.findById(memberId) | ||
.orElseThrow(() -> new EntityNotFoundException("ID에 해당하는 사용자가 존재하지 않습니다.")); | ||
memberRepository.delete(member); | ||
} |
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.
과제하시느라 고생많으셨습니다!!!! 가령 이렇게 예외처리사항이 중복되는 경우 따로 Exception을 만들어서 처리하는 방법이 세미나 내용에 있어서 한번 찾아보시면 좋으실것 같아요..!
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.
이미 좋은 리뷰가 많이 담겨있네요!
은서님 과제 PR 날리는 과정이 어려우셨을텐데 ㅠㅠ 과제 제출하느라 고생하셨습니다!
앞으로도 어려운 부분이 있으면 언제나 불러주세요!!🤍 그리고 코드리뷰 바탕으로 보충할 부분을 찾아봐도 좋을 것 같아요!
화이팅입니다-!
이 주의 과제
Post를 생성하는 API를 구현
사용자는 블로그 ID를 헤더를 통해 제공하고, Post의 내용은 Request Body를 통해 받음
Post를 생성하는 서비스 로직
블로그를 찾고, 새로운 Post를 생성한 후 저장
Post를 생성하기 위한 DTO
Post 생성 성공 메시지를 추가
요구사항 분석
API명세서
[POST] 블로그 글 작성
POST 블로그 글 작성
블로그에 글을 작성하는 API
Path Parameter
Request Header
Request Body
3. Response
✨ Response Body
구현 고민 사항
질문있어요!