[refactor] 아카이브를 수정하는 API 기능의 내부구현을 변경한다. #271
Conversation
…to refactor/256-archive-save
gudonghee2000
requested review from
azxca1731,
chanhl22,
doho-ho and
skullkim
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Component | ||
| @Transactional |
| userVisitPlaces.forEach(userVisitPlace -> userVisitPlace.initArchiveEntity(this)); | ||
| } | ||
|
|
||
| public void update(final String comment, final int starRating, final boolean isVisibleAtItem, |
There was a problem hiding this comment.
isVisibleAtItem이라는 작명은 어떤의미일까요?
| if (!Objects.equals(archiveEntity.getUser().getId(), userId)) { | ||
| throw new ArchiveUnauthorizedException(); | ||
| private void validateNotDuplicateArchive(Long userId, Long itemId) { | ||
| if (archiveRepository.existsByItemIdAndUserId(itemId, userId)) { |
There was a problem hiding this comment.
아카이브 기존에 도메인 객체 가지고 가기로 했는데 애는 왜 없나용?
|
|
||
| boolean existsByItemIdAndUserId(Long itemId, Long userId); | ||
|
|
||
| default ArchiveEntity getByIdAndUserId(final Long id, final Long userId) { |
There was a problem hiding this comment.
이건 왜 default인건가요??
그리고 orElseThrow 로직을 Repository에 둔 이유가 뭔가요???
There was a problem hiding this comment.
이 부분은 어제 논쟁과 유사한 로직인것 같아요!(사실 아직 미완성 PR이여서 수정할 예정입니다ㅎㅎ)
interface는 메서드 선언부만 가질수 있지만 JAVA8(JAVA5일수도 있어요)부터 default 메서드가 되입되면서 interface도 메서드 구현체를 만들수 있게되었는데요!
보통은 ApplicationLayer에서 예외 처리를 한다가 일반적이기에 JPA를 사용하는 경우에는 Reposistory에서 Optional을 반환하고 Service에서 예외 처리를 하는것이 일반적이라고 생각하는데요!
하지만 개인적인 취향으로 Service 객체까지 예외 검증을 올려서 처리할 필요가 있을까와 default 메서드를 씀으로써 Service객체의 추상도가 높아져 코드가 간결해진다. 라는 점때문에 위와 같이 Repository에게 단순 조회에 대한 예외 검증의 Domain 객체 책임을 분산해준다.라는 구성을 해보았어요.
이부분도 굉장히 찬반 논의가 많은 부분이라고 해요!
외람되게는 JAVA8의 interface default 메서드 등장배경은 기존 서비스 회사들이 사용하던 JAVA의 Interface SPEC이 바뀌면서 코드를 수정해야하는 불편함들을 해결해주기 위해 interface에 default 메서드로 구현체를 만들었다고 해요
그래서 JPARepository에서 디폴트 메서드를 쓰는것은 등장배경과는 불일치 하기에 반대하시는 분들도 계시다고 해요!
| @JoinColumn(name = "item") | ||
| private ItemEntity item; | ||
|
|
||
| @OneToMany(mappedBy = "archiveEntity", cascade = {CascadeType.PERSIST, CascadeType.MERGE}, |
There was a problem hiding this comment.
혹시 cascade 설정을 이렇게 해주신 이유가 있나요?
There was a problem hiding this comment.
해당 부분은 다대일 양방향 매핑을 사용하면서 발생한 영속성전이 부분이예요.
Archive와 ArchiveImage 중 연관관계 주인이 ArchiveImage가 되면서 양방향 매핑을 위해 @onetomany에 mappedBy 옵션이 붙게 되었어요
하지만 mappedBy는 읽기 전용이기 때문에 쓰기가 불가능해요.
그래서 CaseCade.Persist 옵션을 주어 Archive를 DB에 영속시킬때 영속성 전이를 통해 ArchiveImages를 같이 영속화 시킴을 명시해주어야 해요
만약, 위와 같이 구성하지 않게되면
ArchiveRepository.save(), ArchiveImageRepository.save() 로직이 두번 발생하게되는데요.
ArchiveImage는 Archive에 생명주기가 종속적인 Entity이기 때문에 영속성 전이를 사용했어요!
[feat] PR을 등록한다.작업 내용
아카이브를 수정하는 API 기능의 내부구현을 변경한다.
Closes #270