Feature : 라벨 기능 구현 #18
Conversation
- handleInvalidLabelException
| public static List<Label> toLabels(List<LabelEntity> entities) { | ||
| return entities.stream() | ||
| .map(LabelEntity::to) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
collect(Collectors.toList());
아래걸로 대체가 됐었던거 같아요
.toList()
| FORBIDDEN(HttpStatus.FORBIDDEN, "COMMON403"), | ||
|
|
||
| // 라벨 에러 | ||
| INVALID_LABEL(HttpStatus.BAD_REQUEST, "LABEL400"), |
There was a problem hiding this comment.
LABEL 4000 어때요 아래가 4001이니까
| assertNotNull(dtos, "Response DTO 리스트는 null이 될 수 없음"); | ||
| assertEquals(2, dtos.size(), "DTO 리스트 사이즈는 2"); | ||
| } | ||
| } |
| @NoArgsConstructor | ||
| @AllArgsConstructor |
There was a problem hiding this comment.
이거 NoArgsConstructor와 AllArgsConstructor는 지양해야 할 Annotaion이라서
@NoArgsConstructor는 @NoArgsConstructor(access=accessLevel.PROTECTED) 이걸로, @AllArgsConstuctor은 최대한 지양하고, Builder를 써야한다는데 저는 @AllArgsConstuctor(access=accessLevel.PRIVATE) 요렇게 합니다 맞는지는 모르겠어요ㅎㅎ
참고 : https://velog.io/@rosa/Lombok-지양해야-할-annotation, https://kcode-recording.tistory.com/342,
There was a problem hiding this comment.
오호 첨 알았어요! 감사합니다~
| public static List<LabelResponseDTO> toResponseDTOList(List<Label> labels) { | ||
| return labels.stream() | ||
| .map(label -> new LabelResponseDTO(label.getImageUrl())) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
다시 찾아보니까 현재 collect(Collectors.toList())는 Java 8 방식에서 사용됐었고
Stream.toList() Java16 부터 생겼다고해요
더 간결하고, 내부적으로 최적화시켜놨다고 합니다!
차이점은 전자는 가변이고 후자는 불변이라고 하네요
There was a problem hiding this comment.
그런 차이가 있었군요 감사합니다!
| return imageUrl; | ||
| } | ||
|
|
||
| public static List<LabelResponseDTO> toResponseDTOList(List<Label> labels) { |
There was a problem hiding this comment.
Label 자신이 DTO로 변환되는 식으로 해도 나쁘지 않을 것 같아요!
public LabelResponseDTO 메서드명() {
}
이런 식으로 작성해서 서비스에서는
List<LabelResponseDTO> response = labels.stream().map(Label::toDTO).toList();해도 괜찮을 것 같아요! 서비스 입장에서 Label의 메서드를 가져와서 이용한다기 보다는, 라벨한테 DTO로 변환하라고 시키는 느낌은 어떨까요?
There was a problem hiding this comment.
오 이 방법이 더 좋은 것 같아요! 수정하겠습니다~
- LABEL400 -> LABEL4001
- ownedCount : 해당 라벨을 소유한 유저 수
- Label 객체 생성 - 소유 인원수가 제한 인원수를 초과하지 않은 경우 - 소유 인원수가 제한 인원수를 초과한 경우 - 소유 인원수 증가 - limitCount가 음수일 경우 예외 발생
- 특정 유저가 소유한 라벨 리스트를 반환한다.
| User user = UserEntity.toUser(userEntity); | ||
|
|
||
| //2. 유저가 해당 라벨을 이미 가지고 있다면 예외 처리 | ||
| List<UserLabel> userLabel = labelRepository.findUserLabelByUserAndLabel(user, label); |
There was a problem hiding this comment.
라벨과 유저를 통해 조회하면 최대 1개만 나오지 않을까 싶은데 List로 받은 이유가 궁금해요!
There was a problem hiding this comment.
혹시 모를 상황을 대비했습니다..ㅎㅎ 혹시 2개 이상이 될 수 있으니까? 그래서 59번째줄에서 리스트의 사이즈를 검사해주고 있어요!
하지만 로직상 2개 이상이 될 상황이 없어서 굳이 리스트로 받지 않아도 될 것 같네요!
| return new LabelResponseDTO(this.labelId, this.imageUrl); | ||
| } | ||
|
|
||
| public boolean isOwnedCountValid() { |
There was a problem hiding this comment.
라벨을 생성할 때, 이 부분을 검증해도 괜찮을 것 같아요!
이미 다 개수가 소진된 라벨이라면, 라벨을 가져오는 데에서부터 예외를 발생시키는 것도 나쁘지않다 생각합니다!
There was a problem hiding this comment.
라벨 도메인 생성 시 검증하면 전체 라벨 조회할 때 선착순 마감 예외가 발생하게 돼서 일단은 서비스에서 검증하도록 했습니다! 나중에 바꿔볼게요!
| //3. label에 최대 인원수와 소유 인원수를 비교한다. | ||
| //3-1. 소유 인원수가 최대 인원수를 넘지 않는다면, 소유 인원수 업데이트 후 UserLabel에 저장 | ||
| if (label.isOwnedCountValid()) { | ||
| labelRepository.updateOwnedCount(label); |
There was a problem hiding this comment.
리포지토리 내에서 라벨의 ownedCount를 상승시켜주고 반영시키는 것 같은데, 상승시키는 건 라벨이 행동하게 하는건 어떨까요?
ownedCount를 증가시키는건 도메인 객체에게 부여하고, 이를 DB에 반영하는 것만 리포지토리에 책임을 주는 것도 괜찮을 것 같아요!
| @ManyToOne | ||
| @JoinColumn(name = "user_id", nullable = false) | ||
| @Cascade(CascadeType.PERSIST) | ||
| private UserEntity user; | ||
|
|
||
| @ManyToOne | ||
| @JoinColumn(name = "label_id", nullable = false) | ||
| @Cascade(CascadeType.PERSIST) | ||
| private LabelEntity label; |
There was a problem hiding this comment.
이거 제 기억 이슈이긴한데 직접참조 허용하는쪽으로 했었나요??
| @PersistenceContext | ||
| private EntityManager entityManager; |
There was a problem hiding this comment.
EntityManager 안쓰고 있어서 지워도 될 것 같아요!
There was a problem hiding this comment.
userEntity를 담을 때 사용하고 있어요! 이 부분은 유저 로직 구현하면 바꿀 예정입니다!!
| LabelEntity labelEntity = entityManager.find(LabelEntity.class, label.getLabelId()); | ||
| UserEntity userEntity = entityManager.find(UserEntity.class, user.getUserId()); |
There was a problem hiding this comment.
JpaRepository랑 EntityManager 둘 다 필드로 존재하는데, EntityManager를 사용하신 이유가 궁금합니다!
| entityManager.persist(userLabelEntity); | ||
|
|
||
| userLabelJpaRepository.save(userLabelEntity); |
There was a problem hiding this comment.
persist를 통해 영속상태가 되고, 트랜잭션이 끝나면 insert 쿼리를 날려줄 것 같아요! persist나 save 둘 중 하나만 해도 괜찮을 것 같아요!
| public Long getUserId() { | ||
| return userId; | ||
| } | ||
|
|
||
| public String getEmail() { | ||
| return email; | ||
| } |
| @Builder | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @AllArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Table(name = "user") |
There was a problem hiding this comment.
저희 끝에 tb붙이는거 아니었나요 저만 붙이는거였나요ㅋㅋㅋㅋㅋㅋㅋㅋ
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋㅋ그런 말 했던 거 기억나는 거 같아요..ㅎㅎㅎㅎㅎ 한번 얘기해봅시다
| this.ownedCount++; | ||
| } | ||
|
|
||
| public LabelResponseDTO toLabelResponseDTO() { |
There was a problem hiding this comment.
DTO에 from을 넣고 LabelResponseDTO.from(label)
LabeResponseDTO 안에
public static LabelResponseDTO from(Label label) {
return new LabelResponseDTO(
label.getLabelId(),
label.getImageUrl()
)
}
이렇게 해서 DTO에게 변환을 위임해주는건 어떤가요
| LabelEntity labelEntity = labelJpaRepository.findByIdWithLock(labelId) | ||
| .orElseThrow(() -> new InvalidLabelException("라벨 ID " + labelId + " 에 해당하는 라벨이 존재하지 않습니다.")); | ||
| entityManager.persist(labelEntity); | ||
| return LabelEntity.toLabel(labelEntity); |
There was a problem hiding this comment.
return labelJpaRepository.findByIdWithLock(labelId)
.map(LabelEntity::toLabel)
.orElseThrow(() -> new InvalidLabelException("라벨 ID " + labelId + " 에 해당하는 라벨이 존재하지 않습니다."))
스트림 쓰면 깔끔해지긴 해요! 취향차이 일 것 같습니다!
| @Transactional | ||
| public List<LabelResponseDTO> findAllLabels() { | ||
| List<Label> labels = labelRepository.findAllLabels(); | ||
| return labels.stream().map(Label::toLabelResponseDTO).toList(); |
There was a problem hiding this comment.
제가 제안드린대로 한다면 LabelResponseDTO::from 이 들어가게 됩니다!
|
|
||
| //2. 유저가 해당 라벨을 이미 가지고 있다면 예외 처리 | ||
| List<UserLabel> userLabel = labelRepository.findUserLabelByUserAndLabel(user, label); | ||
| if (userLabel.size() >= 1) throw new DuplicateLabelException("이미 발급받은 라벨입니다."); |
There was a problem hiding this comment.
건하님 의견대로 가면 userLabel.size() >= 1 보다는 label 안에 검증을 넣어서 isXXX 형식으로 받아오는 것도 괜찮을거 같아요
|
📢 기능 설명
연결된 issue
#1
✅ 체크리스트
ETC