deleteCartItem에서 빈장바구니 정리로직추가 reqeustdto에 카멜케이스로 변경 에러코드 통일 #49
Conversation
Walkthrough카트 도메인 리팩토링으로 필드명을 snake_case에서 camelCase로 표준화하고, ORM 매핑을 명확화하며, 검증 제약을 추가하고, API 문서를 간소화하며, 빈 카트 자동 정리 로직을 도입했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/campustable/be/domain/cart/controller/CartControllerDocs.java (1)
16-16: 문서와 실제 필드명이 불일치합니다.문서에
menu_id로 표기되어 있지만, 실제CartRequest의 필드명은menuId로 변경되었습니다. 클라이언트가 잘못된 필드명을 사용하게 될 수 있습니다.🔎 제안하는 수정사항
description = """ ### 요청 파라미터 - - `menu_id` (Long, required): 장바구니에 담을 메뉴 ID + - `menuId` (Long, required): 장바구니에 담을 메뉴 ID - `quantity` (int, required): 담을 수량 - 1 이상: 해당 수량으로 장바구니에 추가 또는 수정 - 0: 해당 메뉴를 장바구니에서 삭제
🧹 Nitpick comments (4)
src/main/java/com/campustable/be/global/common/SecurityUtil.java (1)
26-30: 예외 발생 시 원인 정보를 로깅해야 합니다.
NumberFormatException발생 시 원본 예외 정보와 잘못된 값을 로깅하지 않아 디버깅이 어렵습니다. 어떤 값이 파싱에 실패했는지 기록하는 것이 좋습니다.🔎 제안하는 수정사항
try { return Long.parseLong(authentication.getName()); } catch (NumberFormatException e) { + log.error("사용자 ID 파싱 실패. 값: {}", authentication.getName(), e); throw new CustomException(ErrorCode.USER_NOT_FOUND); }src/main/java/com/campustable/be/domain/cart/service/CartService.java (1)
75-82: 중복된 빈 장바구니 정리 로직을 별도 메서드로 추출하는 것이 좋습니다.
updateCartItem과deleteCartItem메서드에서 동일한 빈 장바구니 정리 로직이 반복됩니다. 별도 메서드로 추출하면 유지보수성이 향상되고 일관성이 보장됩니다.🔎 제안하는 리팩토링
+private void deleteCartIfEmpty(Cart cart) { + if (cartItemRepository.findByCart(cart).isEmpty()) { + cart.getUser().setCart(null); + cartRepository.delete(cart); + } +} + public CartResponse updateCartItem(Long menuId, int quantity) { // ... 기존 로직 ... cartItemRepository.flush(); List<CartItemDto> cartItems = cartItemRepository.findByCart(cart).stream(). map(CartItemDto::new). toList(); if (cartItems.isEmpty()) { - cart.getUser().setCart(null); - cartRepository.delete(cart); + deleteCartIfEmpty(cart); return CartResponse.builder() .items(List.of()) .totalPrice(0) .build(); } // ... 나머지 로직 ... } public void deleteCartItem(Long cartItemId) { // ... 기존 로직 ... Cart cart = cartItem.getCart(); cartItemRepository.delete(cartItem); cartItemRepository.flush(); - if (cartItemRepository.findByCart(cart).isEmpty()) { - cart.getUser().setCart(null); - cartRepository.delete(cart); - } + deleteCartIfEmpty(cart); }Also applies to: 111-114
src/main/java/com/campustable/be/domain/cart/entity/Cart.java (1)
29-29: 필드명을 camelCase로 통일하는 것을 고려해 주세요.PR 목표가 camelCase로의 통일인데,
cart_id필드는 여전히 snake_case를 사용하고 있습니다.cartId로 변경하되@Column(name = "cart_id")를 유지하면 DB 컬럼명은 그대로 두고 자바 네이밍 규칙을 따를 수 있습니다.🔎 제안하는 수정사항
@Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Column(name = "cart_id") -private Long cart_id; +private Long cartId;이 경우
CartService의 91, 157번 라인에서cart.getCart_id()호출도 함께 수정이 필요합니다.src/main/java/com/campustable/be/domain/cart/controller/CartControllerDocs.java (1)
28-28: 응답 설명을 더 명확하게 작성해 주세요."cartId가 포함되지 않을 수 있습니다"라는 표현이 모호합니다. 실제로
CartService코드를 보면 장바구니가 비워질 때cartId가 포함되지 않으므로, "포함되지 않습니다"로 명확히 작성하는 것이 좋습니다.🔎 제안하는 수정사항
### 특징 - 수량을 0으로 전달하면 해당 메뉴는 삭제됩니다. -- **장바구니가 비워지면 장바구니 엔티티가 자동 삭제**됩니다. (이 경우 응답에 `cartId`가 포함되지 않을 수 있습니다.) +- **장바구니가 비워지면 장바구니 엔티티가 자동 삭제**됩니다. (이 경우 응답에 `cartId`가 포함되지 않습니다.)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/campustable/be/domain/cart/controller/CartController.javasrc/main/java/com/campustable/be/domain/cart/controller/CartControllerDocs.javasrc/main/java/com/campustable/be/domain/cart/dto/CartRequest.javasrc/main/java/com/campustable/be/domain/cart/entity/Cart.javasrc/main/java/com/campustable/be/domain/cart/entity/CartItem.javasrc/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.javasrc/main/java/com/campustable/be/domain/cart/service/CartService.javasrc/main/java/com/campustable/be/global/common/SecurityUtil.java
🔇 Additional comments (8)
src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java (1)
14-18: LGTM!포맷팅 정리로 코드가 깔끔해졌습니다.
src/main/java/com/campustable/be/domain/cart/service/CartService.java (2)
106-114: 빈 장바구니 자동 정리 로직이 올바르게 구현되었습니다.카트 아이템 삭제 후 장바구니가 비었을 때 자동으로 정리하는 로직이 적절하게 추가되었습니다.
flush()를 호출하여 DB와 동기화한 후 빈 장바구니를 확인하고, 사용자와의 연관관계를 끊은 뒤 삭제하는 흐름이 정확합니다.
109-114: 높은 동시성 환경에서의 동작을 검증해 주세요.두 사용자가 동시에 같은 장바구니의 서로 다른 마지막 아이템들을 삭제할 경우, 드물게 빈 장바구니가 남을 가능성이 있습니다.
@Transactional로 격리가 보장되지만, 트래픽이 높은 환경에서는 트랜잭션 타이밍에 따라findByCart조회 시점에 아이템이 남아있는 것으로 보일 수 있습니다.현재 코드는 일반적인 상황에서 문제없지만, 높은 동시성이 예상된다면 추가 검증이나 낙관적 락 고려가 필요할 수 있습니다.
src/main/java/com/campustable/be/domain/cart/controller/CartController.java (1)
38-38: LGTM!
@GetMapping("")으로의 변경이 적절합니다. Spring에서""와"/"는 동일하게 처리되며, 코드가 간결해졌습니다.src/main/java/com/campustable/be/domain/cart/entity/Cart.java (1)
32-32: LGTM!
@JoinColumn(name = "user_id")를 추가하여 ORM 매핑을 명시적으로 지정한 것은 좋은 개선입니다. 외래 키 컬럼명이 명확해져 가독성이 향상되었습니다.src/main/java/com/campustable/be/domain/cart/entity/CartItem.java (1)
37-38: LGTM! 엔티티 레벨 검증이 추가되었습니다.
quantity필드에@Min(value = 0)제약을 추가하여 데이터 무결성이 향상되었습니다. 이는 persist/merge 시점에 검증이 수행됩니다.더 완벽한 검증을 위해서는
CartRequestDTO에도 동일한 검증을 추가하는 것이 좋습니다 (앞서CartRequest.java리뷰 참조).src/main/java/com/campustable/be/domain/cart/controller/CartControllerDocs.java (2)
26-28: LGTM! 빈 장바구니 정리 로직이 문서에 잘 반영되었습니다.장바구니가 비워질 때 자동으로 삭제되는 로직이 API 문서에 명확히 설명되어 있어, 클라이언트 개발자가 예상치 못한 동작에 대비할 수 있습니다.
Also applies to: 77-80
31-32: LGTM! 에러 코드가 일관되게 통일되었습니다.모든 엔드포인트에서 에러 코드가 명확하고 일관되게 정리되었습니다.
MENU_NOT_FOUND,USER_NOT_FOUND,CART_NOT_FOUND,CART_ITEM_NOT_FOUND,ACCESS_DENIED등이 적절하게 문서화되었습니다.Also applies to: 47-48, 65-66, 83-84
| @PostMapping("/items") | ||
| public ResponseEntity<CartResponse> addOrUpdateItems(@RequestBody CartRequest request) { | ||
| CartResponse response = cartService.updateCartItem(request.getMenu_id(), request.getQuantity()); | ||
| CartResponse response = cartService.updateCartItem(request.getMenuId(), request.getQuantity()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
LGTM! 하지만 요청 검증 추가가 필요합니다.
CartRequest의 필드명 변경에 맞춰 getMenuId() 호출이 올바르게 수정되었습니다. 다만 @Valid 어노테이션이 없어 요청 검증이 수행되지 않습니다.
🔎 제안하는 수정사항
@PostMapping("/items")
-public ResponseEntity<CartResponse> addOrUpdateItems(@RequestBody CartRequest request) {
+public ResponseEntity<CartResponse> addOrUpdateItems(@Valid @RequestBody CartRequest request) {
CartResponse response = cartService.updateCartItem(request.getMenuId(), request.getQuantity());
return ResponseEntity.ok(response);
}이를 위해 CartRequest에도 검증 어노테이션이 추가되어야 합니다 (앞서 코멘트 참조).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/campustable/be/domain/cart/controller/CartController.java
around line 31, the controller invokes cartService.updateCartItem(...) but the
CartRequest parameter is not annotated for validation; add @Valid to the
controller method parameter (e.g., public ResponseEntity<?>
updateCart(@RequestBody @Valid CartRequest request)) so Spring MVC performs bean
validation, and ensure CartRequest has the appropriate
javax.validation/jakarta.validation annotations on its fields (e.g., @NotNull,
@Min) to enforce the required constraints.
| public class CartRequest { | ||
|
|
||
| private Long menu_id; | ||
| private Long menuId; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
필수 검증 어노테이션 추가가 필요합니다.
menuId 필드에 @NotNull 검증 어노테이션이 없어 null 값이 서비스 레이어로 전달될 경우 NPE가 발생할 수 있습니다. 컨트롤러에서 @Valid 또는 @Validated와 함께 사용하여 요청 시점에 검증하는 것이 좋습니다.
🔎 제안하는 수정사항
+import jakarta.validation.constraints.NotNull;
+
@Getter
public class CartRequest {
- private Long menuId;
+ @NotNull(message = "메뉴 ID는 필수입니다")
+ private Long menuId;
private int quantity;
}컨트롤러에서도 @Valid 어노테이션을 추가해야 합니다:
public ResponseEntity<CartResponse> addOrUpdateItems(@Valid @RequestBody CartRequest request) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Long menuId; | |
| import jakarta.validation.constraints.NotNull; | |
| @Getter | |
| public class CartRequest { | |
| @NotNull(message = "메뉴 ID는 필수입니다") | |
| private Long menuId; | |
| private int quantity; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/campustable/be/domain/cart/dto/CartRequest.java around line
9, the menuId field lacks a @NotNull validation annotation which allows null to
propagate and can cause NPEs; add the @NotNull annotation to the menuId field
(and the corresponding import, e.g., javax.validation.constraints.NotNull) and
ensure the controller endpoint accepts validated requests by annotating the
request parameter with @Valid (or annotate the controller class with @Validated)
so validation runs before the service layer is invoked.
|
|
||
| private Long menu_id; | ||
| private Long menuId; | ||
| private int quantity; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
수량 필드에 검증 제약이 필요합니다.
quantity 필드에 음수 값이 전달될 경우를 방지하는 검증이 없습니다. 서비스 로직에서 0은 삭제, 양수는 추가/수정으로 처리하지만 음수는 명시적으로 처리되지 않습니다.
🔎 제안하는 수정사항
+import jakarta.validation.constraints.Min;
+
@Getter
public class CartRequest {
private Long menuId;
+ @Min(value = 0, message = "수량은 0 이상이어야 합니다")
private int quantity;
}🤖 Prompt for AI Agents
In src/main/java/com/campustable/be/domain/cart/dto/CartRequest.java around line
10, the quantity field lacks validation and can accept negative values; add a
validation constraint such as javax.validation.constraints.Min(0) or
javax.validation.constraints.PositiveOrZero to the quantity field, import the
corresponding annotation, and ensure the DTO is validated in the
controller/service boundary (e.g., use @Valid on controller method parameters)
so negative quantities are rejected at request validation time.
✨ 변경 사항
deleteCartItem에서 빈장바구니 정리로직추가 reqeustdto에 카멜케이스로 변경 에러코드 통일
✅ 테스트
Summary by CodeRabbit
릴리스 노트
개선사항
문서
✏️ Tip: You can customize this high-level summary in your review settings.