Conversation
…to refactor/256-archive-save
gudonghee2000
requested review from
azxca1731,
chanhl22,
doho-ho and
skullkim
| @ApiOperation(value = "신규 아카이브 등록") | ||
| public ArchiveInfo saveArchive( | ||
| @ApiParam(value = "등록할 아카이브 데이터", required = true) @RequestBody ArchiveRequest request) { | ||
| @ApiParam(value = "등록할 아카이브 데이터", required = true) @RequestBody ArchiveCreateRequest request) { |
| private ItemEntity item; | ||
|
|
||
| @OneToMany(mappedBy = "archiveEntity", cascade = CascadeType.PERSIST) | ||
| private List<ArchiveImageEntity> archiveImages; |
There was a problem hiding this comment.
흠 PERSIST가 맞을까요? 이걸로 하신 이유가 있을까요?
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이기 때문에 영속성 전이를 사용했어요!
| private ArchiveEntity archiveEntity; | ||
|
|
||
| public void setArchiveEntity(ArchiveEntity archiveEntity) { | ||
| public void initArchiveEntity(ArchiveEntity archiveEntity) { |
There was a problem hiding this comment.
이거 이름이 set이 아닌 init으로 바꾼이유가 뭘까용?
There was a problem hiding this comment.
개인적으로 set이라는 네이밍을 좋아하지 않아서 변경해보았어요~
There was a problem hiding this comment.
장단점이 뭘까요? 왜 set이라는 네이밍을 좋아하지 않으신가용?
| return ArchiveEntity.builder() | ||
| .comment(this.getComment()) | ||
| .starRating(this.getStarRating()) | ||
| .isVisibleAtItem(this.isVisibleAtItem()) |
There was a problem hiding this comment.
이거 이름이 왜 isVisibleAtItem이에요? 무슨 뜻인가용?
There was a problem hiding this comment.
해당 필드명이 isVisibleAtItem이예요.
기존에 설정되어 있던 네이밍인데요. 아카이브 공개설정에 대한 옵션인것 같아요~
지금 보면 네이밍에 item이 붙어있어 헷갈리기도 하네요!
There was a problem hiding this comment.
AtItem부터는 빼도 되지 않을까 싶긴하네요~
|
|
||
| return archiveAggregateConverter.convertArchiveInfo(archiveEntity, userResponse, archiveImageEntities, | ||
| userVisitPlaceEntities); | ||
| private void validateNotDuplicateArchive(Long userId, Long itemId) { |
There was a problem hiding this comment.
이거 validate은 왜 domain에 없고 여기에 있나용?
There was a problem hiding this comment.
해당 validate는 Repository 접근이 필요한 검증인데요.
이 부분을 domain에 옮긴다면 함수형 인터페이스나 Repository 빈을 파라미터로 domain 객체에게 전달해주어야 하는데
이미 Service가 Repository를 알고있기때문에 검증을 위해 domain에게 Repository 빈을 넣어주는것이 불필요하다 생각되어 Service에서 처리하도록 하였어요!
There was a problem hiding this comment.
흠 제가 도메인을 서비스가 아닌 이런 도메인 객체로 크게 가져가본 적이 없어서 질문이 이상할 수 도 있어요~
도메인에서 validate할 수 있는 것, 없는 것이 나누어지면 굳이 도메인 객체로서 가져갈 필요가 있나요?
| import com.kilometer.exception.KilometerErrorCode; | ||
| import com.kilometer.exception.KilometerException; | ||
|
|
||
| public class UserNotFoundException extends KilometerException { |
There was a problem hiding this comment.
이부분 만들어서 같이 pr에 반영하겠습니다🔥
2 participants
[feat] PR을 등록한다.작업 내용
Archive와ArchiveImages,UserVisitPlace엔티티객체에 대한 다대일 양방향 연관관계 생성주의사항
Closes #256