-
Notifications
You must be signed in to change notification settings - Fork 3
[#26] 장바구니 도메인 추가 #44
base: feature/39
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- PUT mapping URI 설계 변경 - init 메서드와 addItem 메서드 통합 - 동시성 이슈 관련 수정
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package me.jjeda.mall.cart.domain; | ||
|
||
public enum CartModifyType { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요청의 내용을 TYPE으로 분기하게 되면 API의 설계가 잘못됐을 가능성도 생각해볼 수 있습니다. 위의 리뷰를 참고하셔서 수정해보시면 좋을 것 같습니다~ |
||
ADD, REMOVE | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
import me.jjeda.mall.cart.domain.CartItem; | ||
import me.jjeda.mall.cart.repository.CartRedisRepository; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
import javax.persistence.EntityNotFoundException; | ||
import java.util.List; | ||
|
@@ -14,32 +15,36 @@ | |
public class CartService { | ||
private final CartRedisRepository cartRedisRepository; | ||
|
||
public Cart initCart(String id) { | ||
if (!cartRedisRepository.existsById(id)) { | ||
return cartRedisRepository.save(Cart.of(id)); | ||
} | ||
return getCart(id); | ||
} | ||
|
||
@Transactional(readOnly = true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SQL이 1개만 나가게되는데 트랜잭션을 걸어준 이유가 있을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 트랜잭션에 대한 이해가 부족했네요..ㅎㅎ |
||
public Cart getCart(String id) { | ||
return cartRedisRepository.findById(id).orElseThrow(EntityNotFoundException::new); | ||
} | ||
|
||
@Transactional | ||
public Cart addItem(String id, CartItem cartItem) { | ||
Cart cart = getCart(id); | ||
final Cart cart; | ||
|
||
if (!cartRedisRepository.existsById(id)) { | ||
cart = Cart.of(id); | ||
} else { | ||
cart = getCart(id); | ||
} | ||
|
||
List<CartItem> cartItemList = cart.getCartItemList(); | ||
cartItemList.add(cartItem); | ||
return cartRedisRepository.save(cart); | ||
} | ||
|
||
@Transactional | ||
public Cart removeItem(String id, CartItem cartItem) { | ||
Cart cart = getCart(id); | ||
final Cart cart = getCart(id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. final을 걸어주는건 괜찮지만 이 변수에만 걸어준 이유가 있을까요? 컨벤션은 일관적이면 좋을 것 같아서요~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
List<CartItem> cartItemList = cart.getCartItemList(); | ||
cartItemList.remove(cartItem); | ||
|
||
return cartRedisRepository.save(cart); | ||
} | ||
|
||
@Transactional | ||
public void deleteCart(String id) { | ||
cartRedisRepository.delete(getCart(id)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런식으로 나누기보다는
/carts/items
이런식의 도메인구조면 더 역할나누기가 수월할 것 같습니다~