[FEAT] 쿠폰 API 구현 - Redis Spin Lock, Pub/sub Lock #22
Conversation
| public ResponseEntity<CouponResponse> addCoupon(@RequestBody CouponRequest couponRequest) { | ||
| CouponResponse couponResponse = couponCommandService.saveCoupon(couponRequest); | ||
|
|
||
| return new ResponseEntity<>(couponResponse, HttpStatus.CREATED); |
There was a problem hiding this comment.
P5
return ResponseEntity.status(HttpStatus.CREATED)
.body(response);
저는 메소드 체이닝을 제공해주는 경우에는 메소드 체이닝을 이용하는 편이긴 합니당
|
|
||
| public void removeCoupon(int quantity){ | ||
| if(amount - quantity < 0){ | ||
| throw new IllegalArgumentException(STOCK_ZERO); |
There was a problem hiding this comment.
P5
if (amount < quantity) 가 더 가독성이 좋을 것 같습니당
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
P5
readOnly = true 옵션을 사용하지 않는 경우엔 그냥 @transactional 로 합쳐도 될 것 같습니당
There was a problem hiding this comment.
P4
어차피 CommandService이기 때문에 readOnly를 적용할 일은 없어서 저도 그냥 @transactional으로 합치는 게 좋아보입니다!
There was a problem hiding this comment.
그렇네요 하나로 합치면 좋겠네요 감사합니다 !
|
|
||
| User user = userRepository.findById(userId).orElseThrow(() -> new IllegalArgumentException(NOTFOUND_USER)); | ||
| Coupon coupon = couponRepository.findById(couponId).orElseThrow(() -> new IllegalArgumentException(NOTFOUND_COUPON)); //쿠폰 찾기 | ||
|
|
There was a problem hiding this comment.
P4
코드가 길어지는 경우에는 엔터로 끊어주는 게 가독성이 좋은 것 같습니당
|
|
||
|
|
||
| @Test | ||
| @DisplayName("쿠폰 발급 테스트") |
There was a problem hiding this comment.
P3
DisplayName 을 작성하실 때 명사의 나열보다는 도메인 정책에서 사용하는 언어로 문장의 형태로 작성하시면 테스트만 봐도 로직을 이해할 수 있게 된다고 생각합니당
There was a problem hiding this comment.
저렇게 덩그러니 있는거 보다 확실히 테스트 코드를 자세히 명시해주는게 보는사람 입장에서도 좋겠네요 감사합니다!!
|
|
||
|
|
||
| @PostMapping | ||
| public ResponseEntity<CouponResponse> addCoupon(@RequestBody CouponRequest couponRequest) { |
There was a problem hiding this comment.
P2
@Valid 어노테이션이 있어야 DTO 검증이 적용될 것 같습니다!
There was a problem hiding this comment.
까먹고 빼먹었네요 !! 찾아주셔서 감사합니다!
| if(session == null) { | ||
| throw new IllegalArgumentException(NOT_VALIDATION); | ||
| } |
There was a problem hiding this comment.
저는 session == null 을 체크하는 걸 까먹었네요! 나중에 추가하겠습니다!
| private final List<UserCoupon> userCoupons = new ArrayList<>(); | ||
|
|
||
| public Coupon(int amount, DiscountRate discountRate) { | ||
| private LocalDateTime expired; |
There was a problem hiding this comment.
P4
변수명이 expiredAt으로 하면 좋을 것 같습니다!
| public UserCoupon(User user, Coupon coupon) { | ||
| this.user = user; | ||
| this.coupon = coupon; | ||
| this.coupon.removeCoupon(1); |
There was a problem hiding this comment.
P5
이것도 좋은 방법인거 같긴한데
서비스 계층에서 coupon으로 감소해주는 메서드를 만들어주는 것을 만들어 주면 좋을 것 같습니다!
|
|
||
|
|
||
| public UserCouponResponse useCoupon(Long userId, Long couponId) { | ||
| RLock lock = redisson.getLock(String.format("purchase:book:%d", couponId)); // 쿠폰 아이디에 해당하는 분산락 생성 |
There was a problem hiding this comment.
P5
이름이 purchase book인 이유가 따로 있나요?! 쿠폰에 해당하는 네이밍이면 좋을 것 같습니다!!
There was a problem hiding this comment.
관련이름으로 하는게 좋겠네요 !! 감사합니다!
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
P4
어차피 CommandService이기 때문에 readOnly를 적용할 일은 없어서 저도 그냥 @transactional으로 합치는 게 좋아보입니다!
tnals2384
changed the title
📌 과제 설명
👩💻 요구 사항과 구현 내용
✅ 피드백 반영사항
✅ PR 포인트 & 궁금한 점