Skip to content

Feat/week 2#12

Open
scholar-star wants to merge 8 commits intoApptiveDev:정유진from
scholar-star:feat/week-2
Open

Feat/week 2#12
scholar-star wants to merge 8 commits intoApptiveDev:정유진from
scholar-star:feat/week-2

Conversation

@scholar-star
Copy link

변경점 👍

Post에 대한 댓글 기능을 추가했고, username으로 작성자를 처리하는 대신 작성자를 저장하는 Table인 User Table을 하나 만들어 Post와 Reply Table과의 관계를 형성하였습니다!

버그 해결 💊

JSON으로 userID를 담았음에도 불구하고 SQL에서 not null 제약이 있는 userID에 대해 null 값이 들어왔다는 error가 있어서 살펴보았는데, 알고 보니 실수로 @RequestBody를 설정하지 않았어서 그것을 마저 설정하여 문제를 해결했습니다....

테스트 💻

User Table에 사용자 Mock Data를 넣은 뒤, Post를 생성하고 그 Post에 Reply들을 다는 식으로 Test를 진행하였습니다.

스크린샷 🖼

  • 댓글 추가
replyAdd
  • 댓글들 조회
replyGet
  • 댓글 수정
replyUpdate
  • 댓글 삭제
replyDelete

비고 ✏

수정/삭제의 경우 User이 Reply 작성 User와 같은지에 대한 검수가 한 번 들어가야 안정성을 증가시킬 수 있을 거라 판단하고 있지만, 이 방법에 대해서는 JWT Token 등 Client 단의 도움도 있어야 할 것 같아 이를 이루는 방법을 생각하고 있습니다.

Copy link
Contributor

@sinsehwan sinsehwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

연관관계 설정까지 잘 구성해주셨네요! 고생하셨습니다. 테스트 코드도 추가해보면 좋을 것 같습니다!

Comment on lines +23 to +24
@JoinColumn(name = "users_id")
@ManyToOne
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post에는 작성한 멤버가 필수로 들어가니 다음과 같이 nullable 조건을 추가하면 더 좋을 것 같아요!

@JoinColumn(name = "users_id", nullable = false)
@ManyToOne(optional = false)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

미처 추가하지 못한 부분을 알려주셔서 감사합니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 자료를 찾다보니 궁금한 점이 있는데, @NotNull과 nullable=false는 어떠한 차이가 있는 건지 여쭤봐도 될까요?

Comment on lines +40 to +42
@OneToMany(mappedBy = "posts", cascade = CascadeType.REMOVE, orphanRemoval = true)
private List<Replies> replies = new ArrayList<>();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 이건 대댓글 구현 용도로 만들어두신 걸까요? OneToMany 도입 시 list 배열 동시에 관리하기가 어려워서 저는 보통 꼭 필요한 경우가 아니라면 ManyToOne만 사용하는 단방향 연관관계만 적용하는 것을 선호하긴 합니다!

public List<Posts> findAll();
public Optional<Posts> findById(Integer id);
public List<Posts> findByUsername(String username);
public List<Posts> findByUsersId(Long userID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

username대신 unique한 id로 조회하네요! 좋습니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다!

Comment on lines +30 to 31
Posts postEntity = dtoToEntity(postDTO, user);
postEntity.setCreateat(LocalDateTime.now());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setCreateat을 dtoToEntity에서 진행해도 될 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다! 생각해보니 그렇게 한 번에 처리하는 편이 좀 더 나을 것 같군요

Comment on lines +26 to +35
@Transactional
public List<ReplyResponse> replyGetAll(@PathVariable long postId) {
Posts post = postRepository.findById(postId).orElseThrow();
List<Replies> replies = replyRepository.findByPosts(post);
List<ReplyResponse> repliesResponse = new ArrayList<>();
for (Replies reply: replies) {
repliesResponse.add(ReplyResponse.entityToDTO(reply));
}
return repliesResponse;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 로직은 읽기 연산만 수행하네요! 이럴 때는 @transactional(readOnly=true)를 적용하는 게 최적화 측면에서 좋습니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다! @transactional 주석에 대해서도 찾아보는 과정에서 좀 더 잘 알게 되었어요.

Comment on lines +7 to 9
Long userId,
String username,
String content
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

입력값에 대해 검증이 필요할 것 같아요! @notblank 등을 적용해서 유의미한 입력만 받도록 하면 더 좋을 것 같네요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

조언 감사합니다!


@Transactional
public String deleteReply(@PathVariable long postId, long userId) {
Posts post = postRepository.findById(postId).orElseThrow();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예외를 그대로 던지기보다는 orElseThrow(() -> new InvalidInputException("에러 메시지")); 등으로 에러 문맥을 알 수 있는 에러 타입 및 메시지를 전달해주면 더 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그렇게 예외를 던지는 방법이 있었네요! 미처 생각하지 못한 부분을 알려주셔서 감사합니다!

@ExceptionHandler
public ResponseEntity<String> handleDataAccessException(DataAccessException e) {
ResponseEntity<String> response =
new ResponseEntity<>("DB 오류", HttpStatus.INTERNAL_SERVER_ERROR);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return ResponseEntity.internalServerError().body("DB 오류") 이렇게 더 간결하게 작성할 수 있습니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다! 매번 ResponseEntity를 이용해서 쓰는 법만 알다보니, 그렇게 줄여서 쓸 수 있는 방법은 미처 몰랐네요. 리팩토링할 때 반영해보도록 하겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants