참여자: 고세연, 이창하
pr link : https://github.com/GIST-Petition-Site-Project/GIST-petition-server/pull/72
요약
: 청원에 대한 답변을 위한 도메인인 answer 를 리팩토링 하였다.
기획 수정
: 기존 기획된 부분 중 청원 답변 '권한'에 대한 정책에 모호함이 있어 회의를 진행하였다. 회의에서 결정된 사항은 다음과 같다
사용자 분류
- Anonymous = 비로그인
- User = 로그인한 일반 사용자
- Manager = 비대위 등 관리인
- Admin = super key
권한
- Create Answer : M, Ad
- Read Answer : An, U, M, Ad
- Update Answer : M, Ad
- Delete Answer : M, Ad주요 개발 내용
Answer 엔티티가 post/user 의 ID 만 갖고 있게 함 (직접 연관관계 x)
: Comment 엔티티와 같은 구조. jpa 의 연관관계 설정에 바른 이해 없이 사용하는 것이 위험하다고 판단.
AnswerRequestDto 구현
: controller 와 service 사이의 요청사항 전달 시 사용.
answer 도메인 CRUD 로직 변경
: 기존에 통일되지 않은 컨벤션으로 개발된 CRUD 로직 개선 및 테스트 구현
생각할(해본) 거리
spring validation
화이트 스페이스로 이루어져 있거나, 아예 null 값으로 들어온 answer request 에 대해 isRequestBodyVaild 라는 메서드로 체킹한다.
private boolean isRequestBodyValid(AnswerRequestDto answerRequestDto) {
return answerRequestDto.getContent() != null ;
}이렇게 request body를 체크하는 메서드를 'spring validation'를 이용해 구현해보는 건 어떨까?
DTO 의 범위
AnswerRequestDto 를 Answer 생성자에 사용해봤는데, 엔티티가 dto에 의존하는게 옳은가에 대한 생각이 들었음. dto는 어느 범위까지 써도 될까?
참고 : https://tecoble.techcourse.co.kr/post/2021-04-25-dto-layer-scope/
클래스의 책임
answer 의 create/update/delete 로직 중, 요청한 user 가 해당 권한을 가지고 있는 지 누구에게 물어볼 것 인가?
- 리뷰
// by wannte
getter를 사용해 정보를 가지고 와서 비교하기 보다는 객체에 질의를 던지는 방식은 어떨까요?
user.canWriteAnswer(Post post), user.canUpdateAnswer(Answer answer), user.canDeleteAnswer(Answer answer)
post.canAnsweredBy(User user), answer.canUpdatedBy(User user), answer.canDeletedBy(User user)
정도의 메소드 명이 되겠네요!
만약 이 부분의 도입을 하게 된다면, 저랑 은기의 생각은 후자입니다. 저희 구조상 post가 user에 의존(post가 user를 알고 있음:userId를 가지고 있음), answer가 post와 user에 의존(comment가 userId, postId를 가지고 있음) 이렇게 의존성의 방향이 흐릅니다. 만약 전자의 방법을 선택한다면 user가 post와 comment라는 객체를 알게되는 구조가 되기 때문에 양방향 의존성이 흐르기 때문에, 후자의 방법이 더 적합하다 판단했습니다.) - 이 부분에 대해서 만나서 줌으로 설명해도 좋을 것 같습니다!
객체 안에 객체 안에 객체를 계속 타고타고 들어가면서 검증을 진행한다면, 오히려 검증 과정을 확인하기가 불편할 것 같고, 그런 경우에는 지금의 방식으로 구현을 진행하는 것도 좋은 방법이 될 것이라 생각합니다.
반면에, 객체에 메시지를 보내는 방식으로 진행하게 되면 캡슐화를 진행하고 책임을 분리할 수 있을 것이라 생각해요. 이렇게 했을 때, 어떤 장점을 가지게 될지도 이야기 해봤으면 좋겠어요!- 서비스에서 해결
// AnswerService.class
boolean canUpdate(User user, Answer answer) {
Long answerOwner = answer.getUserId();
return answerOwner.equals(user.getId()) && user.getUserRole()==UserRole.MANAGER;
}- user 에게 물어보기
// User.class
boolean canUpdateAnswer(Answer answer) {
return answer.getUserId().equals(this.id) && this.role==UserRole.MANAGER;
}- answer 에게 물어보기
// Answer.class
boolean canUpdatedBy(User user) {
return this.id.equals(user.getId()) && user.getRole()==UserRole.MANAGER;
}테스트 트랜잭션
테스트 코드를 짜던 도중, user 의 role 을 바꿔야 할 일이 생겼다. 그래서 findBy 로 user를 가져와 setter를 이용해 role을 변경하고, 해당 user를 다시 쿼리해왔다.
바뀐 role이 반영되지 않았다!
...
User user = userRepository.findById(managerUserId);
user.setUserRole(UserRole.USER);
...이 부분인데, 트랜잭션과 영속성 컨텍스트 에 대해 공부해야겠다.
OSIV 라는 키워드도 알아보면 좋다고 한다.
(김영한 님의 JPA책 p.578부터 있다고 한다.)
