댓글 API 구현 #56
댓글 API 구현 #56
Conversation
| if (this.parentComment != null) { | ||
| this.parentComment.getCommentList().remove(this); | ||
| } | ||
| this.parentComment = parentComment; | ||
| parentComment.getCommentList().add(this); |
There was a problem hiding this comment.
[진짜 몰라서 하는 질문] 이렇게 remove 하고 add 하는 부분이 왜 필요한가요? 😲
There was a problem hiding this comment.
데이터베이스와, 메모리 상태의 일관성을 유지하기 위해 사용하였습니다.
간단한 예시입니다.
기댓값은 1이지만, 0이 출력됩니다.
void test(){
User user = new User();
user.setId(1L);
user.setName("test");
Post post = new Post();
post.setId(1L);
post.setUser(user);
List<Post> postList = user.getPostList();
System.out.println("postList.size() = " + postList.size());
}remove의 경우, 연관관계상 둘 이상에 포함될 수 없어서 사용하였습니다.
추천키워드: jpa 연관관계 편의 메소드
There was a problem hiding this comment.
아하!
기존의 연관 관계를 끊기 위해서군요 ㅎㅎ 이해 되었습니다~
| SiteUser siteUser = siteUserRepository.getByEmail(email); | ||
| Post post = postRepository.getById(postId); | ||
| Comment comment = commentRepository.getById(commentId); | ||
| validateDeprecated(comment); |
There was a problem hiding this comment.
[제가 이해한게 맞는지 질문]
validateDeprecated() 검증은 댓글의 본문이 없으면(=대댓글이 달린 댓글을 삭제하면) 수정을 할 수 없단 검증이 맞나요?
There was a problem hiding this comment.
'대댓글이 달린 이후 삭제요청되어 null 값이 된 댓글의 내용을 수정할 수 없다.'의 의미입니다.
| @NotBlank(message = "댓글 내용은 빈 값일 수 없습니다.") | ||
| @Size(min = 1, max = 255, message = "댓글 내용은 최소 1자 이상, 최대 255자 이하여야 합니다.") | ||
| String content, | ||
| Optional<Long> parentId // https://www.baeldung.com/java-record-optional-param |
There was a problem hiding this comment.
주석으로 다신 링크에서는 Optional 이 record 와 같이 쓰이면 안된다고 말하고 있는데 왜 여기서 Optional을 쓰셨는지 궁금해요!
There was a problem hiding this comment.
맞습니다,,
record에서 optional 파라미터를 지양하라고 해서 record 타입을 유지하면서 null value를 받을 수 있는 방안에 대해 알아 보았는데요,
Nullable 어노테이션 활용, record 생성자 구현하여 널 허용 등 다양한 방안을 시도해봤지만 실패하였습니다.
record를 유지하며 optional을 사용해야하나, class를 활용해야하나 고민이 많았는데, 해당 포스트와 같이 record 파라미터에는 optional이 어울리지 않는다고 생각하여 우선 class로 수정하여 처리하였습니다.
추후 더 좋은 방안이 있으면 적용해보도록 하겠습니다.
혹시라도 위와 같은 문제를 접해본적이 있으시다면 힌트 주시면 좋을 것 같습니다ㅜㅜ,,!
There was a problem hiding this comment.
많이 고민해보셨네요🥹👍
세원님의 의도를 제가 이해한 바로는,
'어떤 댓글을 작성할 때 대댓글인지 아닌지의 여부를 확인하기 위해' Optional 을 사용하신 것 같아요.
그런데 Long 을 사용하는 것 만으로도 null 을 허용할 수 있기 때문에 Optional<Long> 일 필요가 없다는 생각이 들었어요.
Optional<List<Object>> 와 같은 느낌이랄까요~ 그 자체로 null 이 될 수 있는 것을 Optional 로 감싼 느낌 ㅎㅎ
심지어 Optional 안의 Long 이 null 이 들어올 수 있기 때문에 이런 코드가 생길 것 같아요.
if(request.parentId.isPresent() && request.parentId.get() != null) {}parentId 의 타입을 Optional 이 아니라 Long 으로 지정해서 간단하게 해결해보는건 어떨까요?
There was a problem hiding this comment.
맨 처음에 Long 타입으로 간단하게 해결하려했는데 예외가 발생해서 다른 방안을 찾아보던거였는데,,,,
혹시 몰라서 다시 해보니 동작하네요,,,,ㅜ
해당 방식으로 리팩토링 하였습니다!
| @SpringBootTest | ||
| @ActiveProfiles("local") | ||
| @DisplayName("댓글 레포지토리 테스트") | ||
| public class CommentRepositoryTest { |
There was a problem hiding this comment.
테스트 클래스는 접근 제한자가 public 일 필요가 없다 생각해요!
|
|
||
|
|
||
| @SpringBootTest | ||
| @ActiveProfiles("local") |
There was a problem hiding this comment.
프로파일을 왜 local로 하셨는지 궁금해요,
빠른 테스트를 위해서 테스트를 돌릴 때는 h2 를 데이터베이스로 삼는 등의 설정이 있었는데
그럼에도 local 로 active profile 을 하신 이유가 있으신가요?
There was a problem hiding this comment.
CommentRepository 내부에서 재귀쿼리를 사용하는데,
h2 데이터베이스의 경우 재귀쿼리 사용에서 오류가 발생하여서
불가피하게 local로 설정하였습니다.
There was a problem hiding this comment.
아하... H2 는 재귀쿼리 사용에 제약이 있군요! 새로운 사실 배워갑니다 ㅎㅎ
| @Test | ||
| @Transactional |
There was a problem hiding this comment.
테스트 코드에 왜 Transactional 을 달아주셨나요?
Transactional 어노테이션을 달면 테스트 코드에서 통과하는게 어플리케이션에서 동작하지 않을 위험이 있다고 알고 있어요.
추가) 서비스 테스트에서는 Transactional 을 달지 않으셨더라구요!
세원님이 의도적으로 이렇게 하신 것 같은데,
왜 레포지토리 테스트에서만 이렇게 해주셨는지 궁금합니다🤔
There was a problem hiding this comment.
좋은 키워드를 제시해 주셔서 너무 감사합니다!
우선 저는 주로 데이터베이스와 연동한 테스트를 작성할때는
변경 내용을 롤백시키고, 각각의 테스트를 독립적으로 동작시키기 위해서 Transactional 어노테이션을 활용해왔던 것 같습니다.
이에 레포지토리 테스트에서만 Transactional 어노테이션을 활용하였습니다.
이로써는 충분한 답변이 되지 못한다고 생각하기에 ‘테스트에서의 Transactional’이라는 키워드로 학습해 보았습니다.
기본적으로 Transactional 어노테이션을 달면 실제 서비스 환경에서 다르게 동작할 위험이 합니다.
보편적으로, 실제 서비스에서는 여러개의 트랜잭션 단위로 작업이 이루어지는데, 테스트 메소드에 Transactional 어노테이션을 달아버리면 하나의 트랜잭션으로 묶여서 동작하여 문제가 없을 수 있습니다.
그렇다면 무조건 테스트에서 Transactional을 사용하지 않은게 과연 올바를까요?
Transactional을 사용하지 않게되면 앞서 언급한 문제점들에서 벗어날 수 있지만,
각각의 테스트에 적합한 클린업 코드를 직접 작성해야하며,
클린업 코드를 잘못 작성하게 된다면 각각의 테스트가 독립적으로 동작하지 않게 될 위험성도 존재합니다.
그렇다면 어떻게 하는게 좋을까요?
** 토비, 김영한님의 의견을 참고하고, 제 주관을 담아 작성했습니다.
Transactional 테스트는 테스트 수행 중에 단 한 개의 트랜잭션 경계만 사용이 되고,
그 경계를 테스트 메소드로 확장을 해도 문제가 없는 상황에서만 유효합니다.
따라서 트랜잭션의 경계를 정확히 파악하고, 문제가 없는 경우에는 Transactional을 활용하고,
문제가 있는 경우, 직접 클린업 코드를 작성하여 테스트를 진행하는게 좋을 것 같다고 생각합니다.
결론
해당 키워드에 대해서는 개발자들 사이에서도 의견이 많이 갈리는 주제라고 생각됩니다.
우선은 제가 학습한 내용을 바탕으로 의견을 정리해 보았는데 피드백 주시면 좋을 것 같습니다.
참고자료
토비님 인프런 답변: 테스트에서의-transactional-사용에-대해-질문이-있습니다
토비님 youtube: 테스트에서의-transactional-사용에-대해-질문이-있습니다
유튜브 1시간짜리 영상인데 제법 재밌게 시청한 것 같습니다
궁금하시다면 한번 시청해보셔도 좋을 것 같습니다!! 끝까지봐야 진짜 꿀잼
There was a problem hiding this comment.
많이 공부해보셨네요
세원님은 정말 학구열이 대단한 분 같습니다!!!!!
제 개인적인 생각을 말씀드리면,
Transactional 을 붙이면 테스트 코드에서 고려해야 할 사항들이 너무 많아지더라고요😓
예를 들어, insert 쿼리가 날아가는 시점에서 id 가 만들어진다고 자연스럽게 생각을 하게 되는데
쓰기 지연 때문에 id 가 null로 조회가 된다면? 그 전에 repository 를 flush 해줘야 할 것이고..
또 어떤 테스트 코드에는 붙이고, 다른 곳에는 붙인다면 매번 고려해야 할 사항이 달라지겠죠?
이런 사소한 것들이 인지적 부담이 될 수 있다 생각해서 'Transactional 안쓰기 & 한번에 DB 를 밀어주기' 방법을 선호합니다.
그리고 조금이라도 실수할 수 있는 여지가 있는 것은 피하려고 해요.
저를 믿지 못하기 때문에🙃
하지만 이건 제 주관이니 고려하실 필요는 없고, 세원님이 옳다고 생각하시는 방법대로 작성해주시면 됩니다
제 생각 정리 겸 적어봤습니다 ㅎㅎ
2 participants
관련 이슈
작업 내용
댓글 관련된 API 구현하였습니다.
테스트코드 작성하였습니다.
모든 테스트 정상동작 합니다.
특이 사항
리뷰 요구사항 (선택)