20251107 #17 cart domain crud기능추가 #46
Conversation
401_400_응답의_명확한_구분Test
feat: 개별 메뉴 조회 API 추가 및 menuUrl 필드 검증 강화 #42Test
Walkthrough장바구니 도메인(엔티티, DTO, 리포지토리, 서비스, 컨트롤러)과 보안 유틸리티를 추가하여 사용자 장바구니의 생성, 항목 추가/수정/삭제 및 조회 엔드포인트를 구현하고 에러코드에 장바구니 관련 상수를 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as CartController
participant Service as CartService
participant Security as SecurityUtil
participant MenuRepo as MenuRepository
participant CartRepo as CartRepository
participant ItemRepo as CartItemRepository
participant DB as Database
Client->>Controller: POST /api/cart/items (menu_id, quantity)
Controller->>Service: updateCartItem(menuId, quantity)
Service->>Security: getCurrentUserId()
Security-->>Service: userId
Service->>CartRepo: findByUser(user) / findByUserId(userId)
alt cart 없음
Service->>CartRepo: save(new Cart)
end
Service->>MenuRepo: findById(menuId)
MenuRepo-->>Service: Menu
Service->>ItemRepo: findByCartAndMenu(cart, menu)
alt item 존재
Service->>ItemRepo: update or delete (quantity == 0 -> delete)
else
Service->>ItemRepo: save(new CartItem) (if quantity > 0)
end
Service->>CartRepo: flush / delete cart if empty
Service-->>Controller: CartResponse (items, totalPrice, cartId)
Controller-->>Client: 200 OK / ResponseEntity
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
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: 15
🧹 Nitpick comments (5)
src/main/java/com/campustable/be/domain/cart/entity/CartItem.java (2)
21-24: 생성자의 위치를 필드 선언 뒤로 이동하세요.Java 코딩 컨벤션에 따라 필드 선언을 먼저 하고, 그 다음에 생성자를 배치하는 것이 일반적입니다. 현재 코드는 생성자가 필드 선언보다 먼저 나와 있어 가독성이 떨어집니다.
🔎 수정 제안
@Entity @Getter @Setter @NoArgsConstructor public class CartItem { - public CartItem(Cart cart, Menu menu){ - this.cart = cart; - this.menu = menu; - } @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Column(name= "cart_item_id") private Long cartItemId; @ManyToOne(fetch = FetchType.LAZY) private Menu menu; @ManyToOne(fetch = FetchType.LAZY) private Cart cart; private int quantity; + + public CartItem(Cart cart, Menu menu){ + this.cart = cart; + this.menu = menu; + }
30-34: @joincolumn 어노테이션 추가를 고려하세요.
@ManyToOne관계에서@JoinColumn을 명시하지 않으면 JPA가 기본 명명 규칙을 사용합니다. 데이터베이스 스키마와 일치하도록 명시적으로 컬럼명을 지정하는 것이 좋습니다.🔎 수정 제안
+import jakarta.persistence.JoinColumn; @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "menu_id") private Menu menu; @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "cart_id") private Cart cart;src/main/java/com/campustable/be/global/common/SecurityUtil.java (1)
26-30: 예외 처리의 일관성을 유지하세요.Line 21에서는
CustomException을 사용하지만, Line 29에서는RuntimeException을 사용하여 일관성이 없습니다. 유저 ID 형식 오류도CustomException으로 처리하는 것이 좋습니다.🔎 수정 제안
먼저 ErrorCode에 새로운 에러 코드를 추가:
INVALID_USER_ID_FORMAT(HttpStatus.BAD_REQUEST, "유저 ID 형식이 올바르지 않습니다.")그 다음 SecurityUtil 수정:
try { return Long.parseLong(authentication.getName()); } catch (NumberFormatException e) { - throw new RuntimeException("유저 ID 형식이 올바르지 않습니다."); + log.error("유저 ID 형식이 올바르지 않습니다. authentication.getName(): {}", authentication.getName()); + throw new CustomException(ErrorCode.INVALID_USER_ID_FORMAT); }src/main/java/com/campustable/be/domain/cart/dto/CartItemDto.java (1)
8-10: DTO의 불변성을 위해@Setter제거 권장DTO는 일반적으로 불변(immutable)으로 유지하는 것이 좋습니다.
@Setter를 제거하면 외부에서 값을 변경할 수 없어 데이터 무결성이 보장됩니다.🔎 제안하는 수정 방법
@Getter -@Setter public class CartItemDto {src/main/java/com/campustable/be/domain/cart/service/CartService.java (1)
56-60: 조건문 스타일: 중괄호 사용 권장단일 문장 조건문에도 중괄호를 사용하면 코드 가독성과 유지보수성이 향상됩니다.
🔎 제안하는 수정 방법
if (cartItemOpt.isPresent()) { - if (quantity == 0){ + if (quantity == 0) { cartItemRepository.delete(cartItemOpt.get()); + } else { + cartItemOpt.get().setQuantity(quantity); } - else cartItemOpt.get().setQuantity(quantity); } else {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/campustable/be/domain/cart/controller/CartController.java(1 hunks)src/main/java/com/campustable/be/domain/cart/controller/CartControllerDocs.java(1 hunks)src/main/java/com/campustable/be/domain/cart/dto/CartItemDto.java(1 hunks)src/main/java/com/campustable/be/domain/cart/dto/CartRequest.java(1 hunks)src/main/java/com/campustable/be/domain/cart/dto/CartResponse.java(1 hunks)src/main/java/com/campustable/be/domain/cart/entity/Cart.java(1 hunks)src/main/java/com/campustable/be/domain/cart/entity/CartItem.java(1 hunks)src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java(1 hunks)src/main/java/com/campustable/be/domain/cart/repository/CartRepository.java(1 hunks)src/main/java/com/campustable/be/domain/cart/service/CartService.java(1 hunks)src/main/java/com/campustable/be/domain/user/entity/User.java(2 hunks)src/main/java/com/campustable/be/global/common/SecurityUtil.java(1 hunks)src/main/java/com/campustable/be/global/exception/ErrorCode.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/java/com/campustable/be/domain/cart/entity/Cart.java (3)
src/main/java/com/campustable/be/domain/cart/entity/CartItem.java (1)
Entity(15-41)src/main/java/com/campustable/be/domain/user/entity/User.java (1)
Entity(13-49)src/main/java/com/campustable/be/domain/cart/dto/CartItemDto.java (1)
Getter(8-24)
src/main/java/com/campustable/be/domain/cart/dto/CartItemDto.java (2)
src/main/java/com/campustable/be/domain/cart/dto/CartRequest.java (1)
Getter(6-11)src/main/java/com/campustable/be/domain/cart/dto/CartResponse.java (1)
Getter(8-15)
src/main/java/com/campustable/be/domain/cart/entity/CartItem.java (4)
src/main/java/com/campustable/be/domain/cart/entity/Cart.java (1)
Entity(19-41)src/main/java/com/campustable/be/domain/cart/dto/CartItemDto.java (1)
Getter(8-24)src/main/java/com/campustable/be/domain/cart/dto/CartRequest.java (1)
Getter(6-11)src/main/java/com/campustable/be/domain/cart/dto/CartResponse.java (1)
Getter(8-15)
src/main/java/com/campustable/be/domain/cart/controller/CartController.java (1)
src/main/java/com/campustable/be/global/common/SecurityUtil.java (1)
Slf4j(9-33)
src/main/java/com/campustable/be/domain/cart/dto/CartRequest.java (2)
src/main/java/com/campustable/be/domain/cart/dto/CartItemDto.java (1)
Getter(8-24)src/main/java/com/campustable/be/domain/cart/dto/CartResponse.java (1)
Getter(8-15)
src/main/java/com/campustable/be/domain/cart/dto/CartResponse.java (2)
src/main/java/com/campustable/be/domain/cart/dto/CartItemDto.java (1)
Getter(8-24)src/main/java/com/campustable/be/domain/cart/dto/CartRequest.java (1)
Getter(6-11)
🔇 Additional comments (5)
src/main/java/com/campustable/be/domain/user/entity/User.java (1)
27-28: 장바구니 관계 설정이 적절합니다.User와 Cart 간의 일대일 양방향 관계가 올바르게 구성되었습니다.
CascadeType.ALL과orphanRemoval = true설정으로 User 삭제 시 Cart도 함께 삭제되어 PR 목표와 일치합니다.src/main/java/com/campustable/be/domain/cart/dto/CartResponse.java (1)
8-15: DTO 구조가 적절합니다.CartResponse가 Lombok 어노테이션을 올바르게 사용하고 있으며, 필드 타입도 적절합니다.
src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java (1)
11-18: 리포지토리 메서드가 적절합니다.Spring Data JPA 쿼리 메서드 명명 규칙을 따르고 있으며, 반환 타입도 적절합니다.
src/main/java/com/campustable/be/domain/cart/repository/CartRepository.java (1)
9-15: 리포지토리 구현이 적절합니다.Spring Data JPA 쿼리 메서드가 올바르게 정의되었으며, Optional 반환 타입으로 null-safety를 보장합니다.
src/main/java/com/campustable/be/global/exception/ErrorCode.java (1)
69-70: 장바구니 에러 코드가 적절하게 추가되었습니다.
CART_NOT_FOUND에러 코드가 PR 목표에 맞게 추가되었으며, HttpStatus와 메시지가 적절합니다.
src/main/java/com/campustable/be/domain/cart/controller/CartController.java
Show resolved
Hide resolved
|
|
||
| @LogMonitoringInvocation | ||
| @Override | ||
| @DeleteMapping("/{cartId}") | ||
| public void deleteCart(@PathVariable Long cartId) { | ||
| cartService.deleteCart(cartId); | ||
| } |
There was a problem hiding this comment.
보안 문제: 다른 사용자의 장바구니 삭제 가능
cartId를 직접 받아 삭제하는 방식은 현재 사용자가 해당 장바구니의 소유자인지 검증하지 않습니다. 인증된 사용자라면 누구나 다른 사용자의 장바구니를 삭제할 수 있습니다.
JWT에서 추출한 사용자의 장바구니만 삭제하도록 수정하거나, Service 레이어에서 소유권 검증을 추가해야 합니다.
There was a problem hiding this comment.
토끼 리뷰처럼 장바구니 삭제를 진행할 경우 소유권 검증이 필요해보입니다
어차피 User : Cart 1:1로 연관관계 구현된다면, PK를 받지않고 JWT를 통해 해당 회원의 장바구니를 삭제 가능합니다
Spring Security Context Holder에서 회원정보 꺼내는 방법에 대해서 한번 찾아보세요
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
src/main/java/com/campustable/be/domain/cart/controller/CartController.java
Show resolved
Hide resolved
| @Operation( | ||
| summary = "장바구니 삭제", | ||
| description = """ | ||
| ### 요청 파라미터 | ||
| - `cartId` (Long, required): 삭제할 장바구니 ID | ||
| ### 응답 데이터 | ||
| - 없음 (HTTP 200 OK) | ||
| ### 사용 방법 | ||
| 1. 삭제할 장바구니 ID를 Path Variable로 전달합니다. | ||
| 2. 해당 장바구니가 DB에서 삭제됩니다. | ||
| ### 유의 사항 | ||
| - 장바구니 ID는 실제 존재하는 값이어야 합니다. | ||
| - 장바구니 삭제 시, 해당 장바구니에 속한 모든 아이템도 함께 삭제됩니다. | ||
| - 인증된 사용자만 요청할 수 있습니다. | ||
| ### 예외 처리 | ||
| - `INVALID_REQUEST` (400 BAD_REQUEST): 잘못된 요청 형식 | ||
| """ | ||
| ) | ||
| void deleteCart(@PathVariable Long cartId); |
There was a problem hiding this comment.
보안 문제: 장바구니 소유권 검증 누락
deleteCart API 문서에서 현재 사용자가 해당 장바구니의 소유자인지 검증하는 내용이 없습니다. 실제 구현에서도 소유권 검증이 없다면, 다른 사용자의 장바구니를 삭제할 수 있는 보안 취약점이 발생합니다.
API 설계 시 cartId를 Path Variable로 받는 대신, JWT에서 추출한 사용자 정보를 기반으로 해당 사용자의 장바구니를 삭제하는 방식을 고려해주세요.
🤖 Prompt for AI Agents
In
src/main/java/com/campustable/be/domain/cart/controller/CartControllerDocs.java
around lines 92 to 114, the API docs for deleteCart omit ownership verification;
update the documentation and behavior description to require authentication and
explicit owner check: indicate that the endpoint uses the authenticated user's
ID (from JWT) to identify the cart or that the service layer will validate that
the cartId belongs to the requesting user, and document that attempts to delete
another user's cart return 403 FORBIDDEN; also add a note to prefer using the
authenticated user's context (not an unvalidated path variable) or to explicitly
validate ownership in the controller/service and describe the error responses
(403 for forbidden, 404 for not found).
| @Operation( | ||
| summary = "장바구니 특정 메뉴 삭제", | ||
| description = """ | ||
| ### 요청 파라미터 | ||
| - `cartItemId` (Long, required): 삭제할 장바구니 아이템 ID | ||
| ### 응답 데이터 | ||
| - 없음 (HTTP 200 OK) | ||
| ### 사용 방법 | ||
| 1. 삭제하려는 장바구니 아이템 ID를 Path Variable로 전달합니다. | ||
| 2. 해당 장바구니 아이템이 DB에서 삭제됩니다. | ||
| ### 유의 사항 | ||
| - 장바구니 아이템 ID는 실제 존재하는 값이어야 합니다. | ||
| - 인증된 사용자만 요청할 수 있습니다. | ||
| ### 예외 처리 | ||
| - `INVALID_REQUEST` (400 BAD_REQUEST): 잘못된 요청 형식 | ||
| """ | ||
| ) | ||
| void deleteCartItem(@PathVariable Long cartItemId); |
There was a problem hiding this comment.
보안 문제: 장바구니 아이템 소유권 검증 누락
deleteCartItem API에서도 동일하게 현재 사용자가 해당 아이템의 소유자인지 검증하는 로직이 필요합니다. 타 사용자의 장바구니 아이템을 삭제할 수 있는 문제가 있습니다.
🤖 Prompt for AI Agents
In
src/main/java/com/campustable/be/domain/cart/controller/CartControllerDocs.java
around lines 116 to 137, the API docs and controller method for deleteCartItem
lack enforcement that the authenticated user actually owns the cart item; add
ownership verification in the controller (or better, in the service layer) so
only the owner can delete it: retrieve the current user id from the security
context, load the CartItem by cartItemId, compare its owner/userId to the
authenticated user id, and if they differ throw an appropriate exception (e.g.,
AccessDeniedException or a 403-mapped custom exception); update the controller
to call the service method that performs this check (or move the check into the
service), and adjust tests/docs to reflect that unauthorized deletion returns
403 Forbidden.
| public CartResponse updateCartItem(Long menuId, int quantity) { | ||
|
|
||
| Long userId = SecurityUtil.getCurrentUserId(); // JWT에서 추출한 유저 ID | ||
|
|
||
| Menu menu = menuRepository.findById(menuId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.MENU_NOT_FOUND)); | ||
|
|
||
| User user = userRepository.findById(userId). | ||
| orElseThrow(() -> { | ||
| log.error("addOrIncreaseCartItem userId : {}를 db에서 발견하지못했음",userId); | ||
| return new CustomException(ErrorCode.USER_NOT_FOUND); | ||
| }); | ||
|
|
There was a problem hiding this comment.
음수 수량에 대한 유효성 검증 누락
API 문서에서 "수량은 음수 값을 허용하지 않습니다"라고 명시되어 있지만, 실제 코드에서는 음수 값에 대한 검증이 없습니다. 음수 수량이 들어오면 잘못된 가격 계산이 발생할 수 있습니다.
🔎 제안하는 수정 방법
public CartResponse updateCartItem(Long menuId, int quantity) {
+ if (quantity < 0) {
+ throw new CustomException(ErrorCode.INVALID_REQUEST);
+ }
Long userId = SecurityUtil.getCurrentUserId(); // JWT에서 추출한 유저 ID📝 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.
| public CartResponse updateCartItem(Long menuId, int quantity) { | |
| Long userId = SecurityUtil.getCurrentUserId(); // JWT에서 추출한 유저 ID | |
| Menu menu = menuRepository.findById(menuId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.MENU_NOT_FOUND)); | |
| User user = userRepository.findById(userId). | |
| orElseThrow(() -> { | |
| log.error("addOrIncreaseCartItem userId : {}를 db에서 발견하지못했음",userId); | |
| return new CustomException(ErrorCode.USER_NOT_FOUND); | |
| }); | |
| public CartResponse updateCartItem(Long menuId, int quantity) { | |
| if (quantity < 0) { | |
| throw new CustomException(ErrorCode.INVALID_REQUEST); | |
| } | |
| Long userId = SecurityUtil.getCurrentUserId(); // JWT에서 추출한 유저 ID | |
| Menu menu = menuRepository.findById(menuId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.MENU_NOT_FOUND)); | |
| User user = userRepository.findById(userId). | |
| orElseThrow(() -> { | |
| log.error("addOrIncreaseCartItem userId : {}를 db에서 발견하지못했음",userId); | |
| return new CustomException(ErrorCode.USER_NOT_FOUND); | |
| }); |
🤖 Prompt for AI Agents
In src/main/java/com/campustable/be/domain/cart/service/CartService.java around
lines 34 to 46, the method updateCartItem lacks validation for negative
quantity; add an immediate check after reading the quantity parameter (before
repository calls) that if (quantity < 0) you throw a validation exception (e.g.,
new CustomException(ErrorCode.INVALID_QUANTITY) or an appropriate existing
ErrorCode), and ensure the method returns/aborts thereafter; if
ErrorCode.INVALID_QUANTITY doesn’t exist, add a new error code and message for
"quantity must be non-negative" and update/cover this behavior with a unit test.
| public void deleteCart(Long cartId) { | ||
| Cart cart = cartRepository.findById(cartId) | ||
| .orElseThrow(()-> new CustomException(ErrorCode.CART_NOT_FOUND)); | ||
|
|
||
| cart.setUser(null); | ||
|
|
||
| cartRepository.deleteById(cartId); | ||
| } |
There was a problem hiding this comment.
보안 취약점: 소유권 검증 없이 장바구니 삭제
deleteCart 메서드도 현재 사용자가 해당 장바구니의 소유자인지 확인하지 않습니다. 또한 cart.setUser(null)은 양방향 관계에서 예기치 않은 동작을 유발할 수 있습니다.
🔎 제안하는 수정 방법
public void deleteCart(Long cartId) {
+ Long userId = SecurityUtil.getCurrentUserId();
Cart cart = cartRepository.findById(cartId)
.orElseThrow(()-> new CustomException(ErrorCode.CART_NOT_FOUND));
+
+ if (!cart.getUser().getUserId().equals(userId)) {
+ throw new CustomException(ErrorCode.ACCESS_DENIED);
+ }
- cart.setUser(null);
-
cartRepository.deleteById(cartId);
}🤖 Prompt for AI Agents
In src/main/java/com/campustable/be/domain/cart/service/CartService.java around
lines 99 to 106, the deleteCart method currently deletes a cart without
verifying ownership and calls cart.setUser(null) which can break bidirectional
relations; change it to retrieve the current authenticated user (e.g. from
SecurityContext or injected AuthService), compare the cart.getUser().getId()
with the current user's id and throw a CustomException(ErrorCode.UNAUTHORIZED)
or CART_NOT_FOUND if mismatch, remove the cart.setUser(null) call, and only then
call cartRepository.deleteById(cartId) (or cartRepository.delete(cart)) within a
transactional context to safely remove the entity while preserving JPA
relationships.
src/main/java/com/campustable/be/global/common/SecurityUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/com/campustable/be/global/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
|
|
||
| @LogMonitoringInvocation | ||
| @Override | ||
| @PostMapping("/") |
There was a problem hiding this comment.
조회 로직 같은데 GetMapping으로 되어야할 듯 합니다
|
|
||
| @LogMonitoringInvocation | ||
| @Override | ||
| @DeleteMapping("/{cartId}") | ||
| public void deleteCart(@PathVariable Long cartId) { | ||
| cartService.deleteCart(cartId); | ||
| } |
src/main/java/com/campustable/be/domain/cart/controller/CartController.java
Show resolved
Hide resolved
|
|
||
| @LogMonitoringInvocation | ||
| @Override | ||
| @DeleteMapping("/{cartId}") | ||
| public void deleteCart(@PathVariable Long cartId) { | ||
| cartService.deleteCart(cartId); | ||
| } |
There was a problem hiding this comment.
토끼 리뷰처럼 장바구니 삭제를 진행할 경우 소유권 검증이 필요해보입니다
어차피 User : Cart 1:1로 연관관계 구현된다면, PK를 받지않고 JWT를 통해 해당 회원의 장바구니를 삭제 가능합니다
Spring Security Context Holder에서 회원정보 꺼내는 방법에 대해서 한번 찾아보세요
| @Getter | ||
| public class CartRequest { | ||
|
|
||
| private Long menu_id; |
| import org.springframework.security.core.context.SecurityContextHolder; | ||
|
|
||
| @Slf4j | ||
| public class SecurityUtil { |
There was a problem hiding this comment.
오호 좋습니다
아래 로직처럼 SecurityContextHolder에서 사용자 정보를 꺼내서 처리하면 돼요
모든 곳에서 사용가능합니다
추가로 @AuthenticationPrincipal 어노테이션에 대해서도 한번 찾아보세요
src/main/java/com/campustable/be/global/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/com/campustable/be/domain/cart/service/CartService.java (1)
34-46: 음수 수량 검증이 여전히 누락됨이전 리뷰에서 지적된 음수 수량 검증이 아직 추가되지 않았습니다. 음수 수량이 허용되면 잘못된 총 가격 계산(line 84-86)이 발생하고 데이터 무결성이 손상됩니다.
🔎 제안하는 수정
public CartResponse updateCartItem(Long menuId, int quantity) { + if (quantity < 0) { + throw new CustomException(ErrorCode.INVALID_REQUEST); + } Long userId = SecurityUtil.getCurrentUserId();
🧹 Nitpick comments (4)
src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java (1)
16-16: 불필요한 빈 줄 제거메서드 선언 사이에 불필요한 빈 줄이 있습니다.
🔎 제안하는 수정
Optional<CartItem> findByCartAndMenu(Cart cart, Menu menu); - List<CartItem> findByCart(Cart cart);src/main/java/com/campustable/be/domain/cart/service/CartService.java (3)
75-82: 양방향 관계 수동 조작 제거 권장Line 76의
cart.getUser().setCart(null)호출은 JPA 양방향 관계를 수동으로 조작합니다.Cart엔티티에orphanRemoval=true또는 적절한 cascade 설정이 되어 있다면,cartRepository.delete(cart)호출만으로 충분하며 JPA가 관계를 자동으로 처리합니다.🔎 제안하는 수정
if (cartItems.isEmpty()) { - cart.getUser().setCart(null); cartRepository.delete(cart); return CartResponse.builder()
110-124: 소유권 검증 추가 완료, 관계 조작 제거 권장이전 리뷰에서 지적된 소유권 검증 누락이 수정되었습니다(lines 117-119). 다만 line 121의
cart.getUser().setCart(null)호출은 불필요합니다. JPA cascade 설정이 올바르다면cartRepository.delete(cart)호출만으로 관계가 자동으로 처리됩니다.🔎 제안하는 수정
if (!cart.getUser().getUserId().equals(userId)) { throw new CustomException(ErrorCode.ACCESS_DENIED); } - cart.getUser().setCart(null); - cartRepository.delete(cart);
126-160: 구현이 올바르며 코드 중복 개선 가능메서드 로직이 올바르게 구현되었습니다. 다만 총 가격 계산 로직(lines 142-144)이
updateCartItem메서드(lines 84-86)와 중복됩니다. 선택적으로 private 메서드로 추출하여 재사용할 수 있습니다.🔎 선택적 리팩토링 제안
+ private int calculateTotalPrice(List<CartItemDto> cartItems) { + return cartItems.stream() + .mapToInt(item -> item.getPrice() * item.getQuantity()) + .sum(); + } + public CartResponse updateCartItem(Long menuId, int quantity) { ... - int totalPrice = cartItems.stream() - .mapToInt(item->item.getPrice() * item.getQuantity()) - .sum(); + int totalPrice = calculateTotalPrice(cartItems); ... } public CartResponse getCart(){ ... - int totalPrice = cartItems.stream() - .mapToInt(item->item.getPrice() * item.getQuantity()) - .sum(); + int totalPrice = calculateTotalPrice(cartItems); ... }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/campustable/be/domain/cart/controller/CartController.javasrc/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.javasrc/main/java/com/campustable/be/domain/cart/repository/CartRepository.javasrc/main/java/com/campustable/be/domain/cart/service/CartService.javasrc/main/java/com/campustable/be/global/common/SecurityUtil.javasrc/main/java/com/campustable/be/global/exception/ErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/campustable/be/global/exception/ErrorCode.java
- src/main/java/com/campustable/be/global/common/SecurityUtil.java
- src/main/java/com/campustable/be/domain/cart/controller/CartController.java
🔇 Additional comments (2)
src/main/java/com/campustable/be/domain/cart/repository/CartRepository.java (1)
9-15: 구현이 적절합니다Repository 인터페이스가 올바르게 정의되었으며,
findByUser와findByUserId메서드 모두 서비스 레이어에서 유용하게 사용됩니다.src/main/java/com/campustable/be/domain/cart/service/CartService.java (1)
95-108: 소유권 검증이 올바르게 구현됨이전 리뷰에서 지적된 소유권 검증 누락 문제가 수정되었습니다(lines 102-104). 현재 사용자만 자신의 장바구니 아이템을 삭제할 수 있도록 보안이 강화되었습니다.
src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java
Outdated
Show resolved
Hide resolved
| if (cartItemOpt.isPresent()) { | ||
| if (quantity == 0){ | ||
| cartItemRepository.delete(cartItemOpt.get()); | ||
| } | ||
| else cartItemOpt.get().setQuantity(quantity); | ||
| } else { | ||
| if (quantity > 0) { | ||
| CartItem newItem = new CartItem(cart, menu); | ||
| newItem.setQuantity(quantity); | ||
| cartItemRepository.save(newItem); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
else 블록에 중괄호 추가 필요
Line 60의 else 문에 중괄호가 없어 향후 코드 수정 시 버그가 발생할 수 있습니다. 일관성을 위해 모든 제어문에 중괄호를 사용하는 것이 좋습니다.
🔎 제안하는 수정
if (cartItemOpt.isPresent()) {
if (quantity == 0){
cartItemRepository.delete(cartItemOpt.get());
}
- else cartItemOpt.get().setQuantity(quantity);
+ else {
+ cartItemOpt.get().setQuantity(quantity);
+ }
} else {📝 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.
| if (cartItemOpt.isPresent()) { | |
| if (quantity == 0){ | |
| cartItemRepository.delete(cartItemOpt.get()); | |
| } | |
| else cartItemOpt.get().setQuantity(quantity); | |
| } else { | |
| if (quantity > 0) { | |
| CartItem newItem = new CartItem(cart, menu); | |
| newItem.setQuantity(quantity); | |
| cartItemRepository.save(newItem); | |
| } | |
| } | |
| if (cartItemOpt.isPresent()) { | |
| if (quantity == 0){ | |
| cartItemRepository.delete(cartItemOpt.get()); | |
| } | |
| else { | |
| cartItemOpt.get().setQuantity(quantity); | |
| } | |
| } else { | |
| if (quantity > 0) { | |
| CartItem newItem = new CartItem(cart, menu); | |
| newItem.setQuantity(quantity); | |
| cartItemRepository.save(newItem); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/campustable/be/domain/cart/service/CartService.java around
lines 56 to 67, the else at line 60 lacks braces which risks future bugs; update
the else to use braces around its single statement (wrap the existing
cartItemOpt.get().setQuantity(quantity); line in { } ), and reformat to keep
bracing style consistent with the surrounding if blocks.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java (1)
14-17: 선택사항: 성능 최적화를 위한 인덱스 고려장바구니 조회가 빈번하게 발생할 경우,
CartItem엔티티의cart_id와menu_id컬럼에 데이터베이스 인덱스 추가를 고려해보세요. 이는findByCart와findByCartAndMenu메서드의 성능을 향상시킬 수 있습니다.예시 (CartItem 엔티티에 추가):
@Table(indexes = { @Index(name = "idx_cart_id", columnList = "cart_id"), @Index(name = "idx_cart_menu", columnList = "cart_id, menu_id") })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java
🔇 Additional comments (2)
src/main/java/com/campustable/be/domain/cart/repository/CartItemRepository.java (2)
11-12: LGTM - 리포지토리 선언이 올바릅니다.Spring Data JPA 리포지토리 인터페이스가 올바르게 선언되었습니다.
@Repository어노테이션과JpaRepository<CartItem, Long>확장이 적절합니다.
14-17: 쿼리 메서드가 올바르게 구현되어 있고 CartService에서 사용 중입니다.두 메서드 모두 CartService에서 활발하게 사용되고 있습니다:
findByCartAndMenu: 장바구니에 동일한 메뉴 항목이 이미 존재하는지 확인 (중복 방지)findByCart: 장바구니의 모든 항목을 조회하여 DTO로 변환메서드 이름은 Spring Data JPA 규칙을 올바르게 따르고 있으며, 생성된 쿼리가 의도된 대로 작동합니다.
…b.com//issues/17
✨ 변경 사항
장바구니 추가 삭제 및
User -> Cart -> CartItem <- Menu
Cart 삭제시 관련된 CartItem 삭제되도록 구현하였음
✅ 테스트
Summary by CodeRabbit
새로운 기능
문서
기타
✏️ Tip: You can customize this high-level summary in your review settings.