Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
개요이 PR은 음식 배달 앱의 핵심 비즈니스 도메인 5가지(주소, 메뉴, 주문, 리뷰, 음식점)에 대한 완전한 REST API 계층을 추가합니다. 각 도메인마다 JPA 엔티티, 검증된 요청/응답 DTO, 저장소, 비즈니스 서비스, REST 컨트롤러 및 커스텀 예외를 구현하여 일관된 API 아키텍처를 제공합니다. 변경 사항음식 배달 REST API 도메인 구현
예상 코드 리뷰 난이도🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/kuit/baemin/controller/ReviewController.java`:
- Around line 26-29: The deleteReview controller currently accepts memberId as a
`@RequestParam` which allows callers to spoof authorization; change the
deleteReview method to remove the `@RequestParam` Long memberId parameter and
instead obtain the authenticated user's id from the security context (e.g.,
SecurityContextHolder.getContext().getAuthentication().getPrincipal()/getName()
or a JwtAuthenticationToken) and pass that authenticated id into
reviewService.deleteReview(reviewId, authenticatedMemberId); update the method
signature and any callers accordingly so authorization is enforced from the
authentication context rather than a request parameter.
In `@src/main/java/com/kuit/baemin/domain/order/Order.java`:
- Around line 54-56: Order currently exposes orderItems (Order.orderItems)
without convenience methods to keep the bidirectional relationship in sync with
OrderItem.order, so callers adding/removing items may forget to set the
back-reference and cause FK inconsistencies; add instance methods on Order named
addOrderItem(OrderItem item) and removeOrderItem(OrderItem item) that
respectively set item.assignOrder(this) (or item.setOrder(this)) and add/remove
the item from orderItems (and clear item.order on removal), and add a small
helper on OrderItem (e.g., assignOrder(Order order)) that sets the order field
so both sides are always updated together.
In `@src/main/java/com/kuit/baemin/domain/restaurant/Restaurant.java`:
- Around line 48-50: Add bidirectional convenience methods on Restaurant to keep
Restaurant.menus and Menu.restaurant consistent: implement addMenu(Menu menu) to
check for null, add menu to the Restaurant.menus list if not present and call
menu.setRestaurant(this); implement removeMenu(Menu menu) to remove from the
list and if removed set menu.setRestaurant(null). Use the existing menus field
and the Menu.setRestaurant(...) mutator (or create it if missing) and guard
against duplicate additions and nulls to maintain consistency.
- Around line 32-36: The Restaurant entity uses Float for monetary fields which
risks precision errors; change the types of minOrderAmount and deliveryFee (and
any other money-related fields such as rating/score if present) from Float to
java.math.BigDecimal, update their field declarations in class Restaurant,
adjust any constructors, getters/setters, and builder methods that reference
these fields, and add appropriate JPA column metadata (e.g., `@Column`(precision =
X, scale = Y) or `@Digits`) to preserve DB precision; also update any code that
performs arithmetic on these fields to use BigDecimal operations (add, subtract,
multiply, divide) rather than floating-point math so persistence and
calculations remain accurate.
In `@src/main/java/com/kuit/baemin/dto/request/CreateAddressReq.java`:
- Around line 9-14: The DTO CreateAddressReq validation doesn't match your
entity constraints: add length validation to both roadAddress and detailAddress
by applying `@Size`(max=40) on those fields, and ensure isDefault cannot be null
by either changing its type to primitive boolean or adding `@NotNull` to the
Boolean isDefault field (keeping the default false). Update CreateAddressReq to
include these annotations so inputs are validated before hitting the DB (refer
to the roadAddress, detailAddress, and isDefault fields).
In `@src/main/java/com/kuit/baemin/dto/request/CreateMenuReq.java`:
- Around line 15-16: The price field in CreateMenuReq currently only uses
`@NotNull` and can still accept zero or negative values; update the validation by
adding either `@Positive` (for strictly >0) or `@PositiveOrZero` (if
free/zero-priced menus are allowed) to the private Integer price field in class
CreateMenuReq so requests with invalid prices are rejected; keep the existing
`@NotNull` if desired for clearer error messages and ensure imports for
javax.validation.constraints.Positive or PositiveOrZero are present.
In `@src/main/java/com/kuit/baemin/dto/request/CreateOrderReq.java`:
- Around line 33-34: The CreateOrderReq DTO's quantity field currently only has
`@NotNull` and allows zero or negative values; update the quantity field in class
CreateOrderReq to add a minimum-value validation by annotating it with `@Min`(1)
(retain `@NotNull`) so only positive quantities are accepted, and import
javax.validation.constraints.Min (or the project's validation Min) accordingly.
- Around line 25-26: The items field in CreateOrderReq is only annotated with
`@NotEmpty` so nested OrderItemReq constraints won't be validated; add the `@Valid`
annotation to the items field (i.e., annotate List<OrderItemReq> items with
`@Valid`) so that each OrderItemReq's `@NotNull/`@Valid constraints are propagated
and ensure the corresponding javax.validation/ jakarta.validation.Valid import
is present; target the CreateOrderReq class and the items field when applying
the change.
In `@src/main/java/com/kuit/baemin/dto/request/CreateRestaurantReq.java`:
- Around line 20-23: Change the monetary fields from Float to BigDecimal: update
CreateRestaurantReq fields minOrderAmount and deliveryFee to
java.math.BigDecimal, then propagate the same type change to the Restaurant
entity (field names minOrderAmount, deliveryFee) and to the RestaurantRes DTO;
adjust any constructors/getters/setters and mapping code that reads/writes these
fields (e.g., mapper methods or builders) to use BigDecimal. Also update
database column types (schema/migration) to a fixed-point/decimal type and
ensure JSON (de)serialization supports BigDecimal (Jackson config or
annotations) and keep existing validation annotations like `@NotNull` on
deliveryFee. Ensure imports are updated accordingly.
- Around line 19-23: The DTO CreateRestaurantReq currently only uses `@NotNull`
for the monetary fields minOrderAmount and deliveryFee, which allows negative
values; update the field annotations to add `@PositiveOrZero` (or `@Positive` if
policy requires strictly positive) to both minOrderAmount and deliveryFee so
negative inputs are rejected by validation, and ensure the proper
javax/validation import is present for the chosen annotation.
In `@src/main/java/com/kuit/baemin/service/AddressService.java`:
- Around line 40-46: The getAddresses method currently does a redundant
memberRepository.findById lookup before calling
addressRepository.findAllByMemberId causing two DB hits; remove the
memberRepository.findById(...) call and simply return
addressRepository.findAllByMemberId(memberId).stream().map(AddressRes::from).collect(...)
so only one query is executed, or if explicit member existence must be enforced
replace the findById check with a cheaper existsById(memberId) call to minimize
cost while preserving validation.
In `@src/main/java/com/kuit/baemin/service/MenuService.java`:
- Around line 41-47: The getMenus method does an unnecessary double DB lookup:
restaurantRepository.findById(...) followed by
menuRepository.findAllByRestaurantId(...), causing two queries; either remove
the existence check (delete the restaurantRepository.findById(...) call) so
getMenus only calls
menuRepository.findAllByRestaurantId(restaurantId).stream().map(MenuRes::from).collect(...),
or if restaurant validation is required keep the current code but
document/accept the extra query; update getMenus accordingly and ensure
references to restaurantRepository.findById,
menuRepository.findAllByRestaurantId, and MenuRes::from are the points of
change.
In `@src/main/java/com/kuit/baemin/service/OrderService.java`:
- Around line 46-60: The loop in create order uses menuRepository.findById per
item causing an N+1 query; instead collect all menu IDs from req.getItems(), add
a repository method like findAllByIdIn(List<Long> ids) on MenuRepository, fetch
all menus once, map them by id, then iterate req.getItems() using the map to
look up each Menu (throw MenuException with ErrorStatus.MENU_NOT_FOUND if
missing and ErrorStatus.MENU_NOT_AVAILABLE if menu.getIsAvailable() is false),
compute itemTotal using menu.getPrice() * itemReq.getQuantity(), accumulate
totalPrice, and build OrderItem via OrderItem.builder() into orderItems as
before.
- Around line 73-76: The current loop only mutates the Order side
(order.getOrderItems().add(item)) and breaks the bidirectional contract with
OrderItem, so add a convenience mutator on Order (e.g., addOrderItem(OrderItem
orderItem)) that both adds to this.orderItems and calls
orderItem.setOrder(this), then replace the loop in OrderService to call
order.addOrderItem(item) for each OrderItem to ensure both sides (Order and
OrderItem) are kept consistent.
In `@src/main/java/com/kuit/baemin/service/ReviewService.java`:
- Around line 29-31: Replace domain-specific exceptions with the general
exception pattern: in ReviewService where MemberException and OrderException are
thrown (e.g., the expressions creating new
MemberException(ErrorStatus.MEMBER_NOT_FOUND) and new
OrderException(ErrorStatus.ORDER_NOT_FOUND)), throw new
GeneralException(ErrorStatus.MEMBER_NOT_FOUND) and new
GeneralException(ErrorStatus.ORDER_NOT_FOUND) instead; do the same for the other
occurrences noted (lines around 47-49) so all service-level throws use
GeneralException(ErrorStatus.XXX).
- Around line 27-33: The createReview method currently only checks existence of
Member and Order but does not verify ownership, allowing reviews for others'
orders; after retrieving member (memberRepository.findById(...)) and order
(orderRepository.findById(...)), add a check that order.getMember().getId() (or
order.getMember()) equals req.getMemberId() (or member) and if not throw a
suitable exception (e.g., new OrderException(ErrorStatus.ORDER_NOT_OWNER) or
reuse OrderException/ErrorStatus.ORDER_NOT_FOUND with a clearer status),
preventing creation when the requester is not the order owner; update unit tests
for createReview to cover mismatched memberId/orderId case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d8869d7-e2dd-471a-a12b-a8b7162b5574
📒 Files selected for processing (39)
src/main/java/com/kuit/baemin/controller/AddressController.javasrc/main/java/com/kuit/baemin/controller/MenuController.javasrc/main/java/com/kuit/baemin/controller/OrderController.javasrc/main/java/com/kuit/baemin/controller/RestaurantController.javasrc/main/java/com/kuit/baemin/controller/ReviewController.javasrc/main/java/com/kuit/baemin/domain/Restaurant/Restaurant.javasrc/main/java/com/kuit/baemin/domain/address/Address.javasrc/main/java/com/kuit/baemin/domain/menu/Menu.javasrc/main/java/com/kuit/baemin/domain/order/Order.javasrc/main/java/com/kuit/baemin/domain/orderitem/OrderItem.javasrc/main/java/com/kuit/baemin/domain/restaurant/Restaurant.javasrc/main/java/com/kuit/baemin/domain/restaurant/RestaurantStatus.javasrc/main/java/com/kuit/baemin/domain/review/Review.javasrc/main/java/com/kuit/baemin/dto/request/CreateAddressReq.javasrc/main/java/com/kuit/baemin/dto/request/CreateMenuReq.javasrc/main/java/com/kuit/baemin/dto/request/CreateOrderReq.javasrc/main/java/com/kuit/baemin/dto/request/CreateRestaurantReq.javasrc/main/java/com/kuit/baemin/dto/request/CreateReviewReq.javasrc/main/java/com/kuit/baemin/dto/response/AddressRes.javasrc/main/java/com/kuit/baemin/dto/response/MenuRes.javasrc/main/java/com/kuit/baemin/dto/response/OrderRes.javasrc/main/java/com/kuit/baemin/dto/response/RestaurantRes.javasrc/main/java/com/kuit/baemin/dto/response/ReviewRes.javasrc/main/java/com/kuit/baemin/exception/AddressException.javasrc/main/java/com/kuit/baemin/exception/MenuException.javasrc/main/java/com/kuit/baemin/exception/OrderException.javasrc/main/java/com/kuit/baemin/exception/RestaurantException.javasrc/main/java/com/kuit/baemin/exception/ReviewException.javasrc/main/java/com/kuit/baemin/exception/errorcode/ErrorStatus.javasrc/main/java/com/kuit/baemin/repository/AddressRepository.javasrc/main/java/com/kuit/baemin/repository/MenuRepository.javasrc/main/java/com/kuit/baemin/repository/OrderRepository.javasrc/main/java/com/kuit/baemin/repository/RestaurantRepository.javasrc/main/java/com/kuit/baemin/repository/ReviewRepository.javasrc/main/java/com/kuit/baemin/service/AddressService.javasrc/main/java/com/kuit/baemin/service/MenuService.javasrc/main/java/com/kuit/baemin/service/OrderService.javasrc/main/java/com/kuit/baemin/service/RestaurantService.javasrc/main/java/com/kuit/baemin/service/ReviewService.java
💤 Files with no reviewable changes (1)
- src/main/java/com/kuit/baemin/domain/Restaurant/Restaurant.java
Comment on lines
+26
to
+29
| public ApiResponse<Void> deleteReview( | ||
| @PathVariable Long reviewId, | ||
| @RequestParam Long memberId) { | ||
| reviewService.deleteReview(reviewId, memberId); |
There was a problem hiding this comment.
memberId를 쿼리 파라미터로 받아 권한을 판단하면 삭제 권한이 위조됩니다.
현재는 호출자가 memberId를 임의로 전달할 수 있어, 실질적인 인증 없이 리뷰 삭제가 가능합니다. 삭제 권한은 요청 파라미터가 아니라 인증 컨텍스트(예: JWT의 subject) 기반으로 확인해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/controller/ReviewController.java` around lines
26 - 29, The deleteReview controller currently accepts memberId as a
`@RequestParam` which allows callers to spoof authorization; change the
deleteReview method to remove the `@RequestParam` Long memberId parameter and
instead obtain the authenticated user's id from the security context (e.g.,
SecurityContextHolder.getContext().getAuthentication().getPrincipal()/getName()
or a JwtAuthenticationToken) and pass that authenticated id into
reviewService.deleteReview(reviewId, authenticatedMemberId); update the method
signature and any callers accordingly so authorization is enforced from the
authentication context rather than a request parameter.
Comment on lines
+54
to
+56
| @Builder.Default | ||
| @OneToMany(mappedBy = "order", cascade = CascadeType.ALL) | ||
| private List<OrderItem> orderItems = new ArrayList<>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and check the Order.java file
find . -type f -name "Order.java" | grep -E "domain/order"Repository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 126
🏁 Script executed:
# Find and check the OrderItem.java file
find . -type f -name "OrderItem.java" | grep -E "domain/order"Repository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 134
🏁 Script executed:
cat -n ./src/main/java/com/kuit/baemin/domain/order/Order.javaRepository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 1979
🏁 Script executed:
cat -n ./src/main/java/com/kuit/baemin/domain/orderitem/OrderItem.javaRepository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 1137
양방향 연관관계 편의 메서드가 필요합니다.
Order와 OrderItem 사이의 양방향 관계(Order.orderItems ↔ OrderItem.order)에서 편의 메서드가 없어 객체 그래프 동기화 누락 위험이 있습니다. 서비스 코드에서 order.getOrderItems().add(item)만 수행할 경우 item.order가 설정되지 않아 FK 불일치가 발생할 수 있으므로, addOrderItem/removeOrderItem 편의 메서드로 양쪽을 동시에 동기화하세요.
🔧 제안 코드
`@Builder.Default`
`@OneToMany`(mappedBy = "order", cascade = CascadeType.ALL)
private List<OrderItem> orderItems = new ArrayList<>();
+
+public void addOrderItem(OrderItem orderItem) {
+ this.orderItems.add(orderItem);
+ orderItem.assignOrder(this);
+}
+
+public void removeOrderItem(OrderItem orderItem) {
+ this.orderItems.remove(orderItem);
+ orderItem.assignOrder(null);
+}OrderItem 클래스에도 보조 메서드 추가:
public void assignOrder(Order order) {
this.order = order;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/domain/order/Order.java` around lines 54 - 56,
Order currently exposes orderItems (Order.orderItems) without convenience
methods to keep the bidirectional relationship in sync with OrderItem.order, so
callers adding/removing items may forget to set the back-reference and cause FK
inconsistencies; add instance methods on Order named addOrderItem(OrderItem
item) and removeOrderItem(OrderItem item) that respectively set
item.assignOrder(this) (or item.setOrder(this)) and add/remove the item from
orderItems (and clear item.order on removal), and add a small helper on
OrderItem (e.g., assignOrder(Order order)) that sets the order field so both
sides are always updated together.
Comment on lines
+9
to
+14
| @NotBlank(message = "도로명 주소는 필수입니다.") | ||
| private String roadAddress; | ||
|
|
||
| private String detailAddress; | ||
|
|
||
| private Boolean isDefault = false; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CreateAddressReq.java" -type fRepository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 136
🏁 Script executed:
find . -name "Address.java" -o -name "Address*.java" | grep -i entityRepository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 52
🏁 Script executed:
cat -n ./src/main/java/com/kuit/baemin/dto/request/CreateAddressReq.javaRepository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 475
🏁 Script executed:
find . -type f -name "*.java" | xargs rg "class Address" | head -20Repository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 569
🏁 Script executed:
cat -n ./src/main/java/com/kuit/baemin/domain/address/Address.javaRepository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 1039
요청 DTO 검증이 엔티티 제약과 불일치합니다.
roadAddress와 detailAddress는 길이 제한(40) 검증이 없어 DB 예외로 떨어질 수 있고, isDefault는 Boolean이라 null 입력이 허용됩니다. 엔티티의 nullable=false 제약과도 맞지 않으므로 입력 단계에서 검증을 보강해주세요.
🔧 제안 코드
import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Size;
`@Getter`
public class CreateAddressReq {
`@NotBlank`(message = "도로명 주소는 필수입니다.")
+ `@Size`(max = 40, message = "도로명 주소는 40자 이하여야 합니다.")
private String roadAddress;
+ `@Size`(max = 40, message = "상세 주소는 40자 이하여야 합니다.")
private String detailAddress;
- private Boolean isDefault = false;
+ `@NotNull`(message = "기본 주소 여부는 필수입니다.")
+ private Boolean isDefault = false;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/dto/request/CreateAddressReq.java` around lines
9 - 14, The DTO CreateAddressReq validation doesn't match your entity
constraints: add length validation to both roadAddress and detailAddress by
applying `@Size`(max=40) on those fields, and ensure isDefault cannot be null by
either changing its type to primitive boolean or adding `@NotNull` to the Boolean
isDefault field (keeping the default false). Update CreateAddressReq to include
these annotations so inputs are validated before hitting the DB (refer to the
roadAddress, detailAddress, and isDefault fields).
Comment on lines
+41
to
+47
| public List<MenuRes> getMenus(Long restaurantId) { | ||
| restaurantRepository.findById(restaurantId) | ||
| .orElseThrow(() -> new RestaurantException(ErrorStatus.RESTAURANT_NOT_FOUND)); | ||
| return menuRepository.findAllByRestaurantId(restaurantId).stream() | ||
| .map(MenuRes::from) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
불필요한 중복 조회를 제거하세요.
restaurantRepository.findById로 음식점 존재 여부만 확인하고, 바로 다음 줄에서 menuRepository.findAllByRestaurantId를 호출하여 2번의 DB 쿼리가 발생합니다. 메뉴 목록이 비어있을 때와 음식점이 없을 때를 구분해야 하는 명확한 비즈니스 요구사항이 없다면, 음식점 검증 쿼리를 제거하는 것을 고려하세요.
♻️ 제안하는 리팩토링
음식점 검증이 반드시 필요하지 않다면:
public List<MenuRes> getMenus(Long restaurantId) {
- restaurantRepository.findById(restaurantId)
- .orElseThrow(() -> new RestaurantException(ErrorStatus.RESTAURANT_NOT_FOUND));
return menuRepository.findAllByRestaurantId(restaurantId).stream()
.map(MenuRes::from)
.collect(Collectors.toList());
}음식점 검증이 반드시 필요하다면 현재 코드를 유지하되, 이 비용을 인지하고 있어야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/service/MenuService.java` around lines 41 - 47,
The getMenus method does an unnecessary double DB lookup:
restaurantRepository.findById(...) followed by
menuRepository.findAllByRestaurantId(...), causing two queries; either remove
the existence check (delete the restaurantRepository.findById(...) call) so
getMenus only calls
menuRepository.findAllByRestaurantId(restaurantId).stream().map(MenuRes::from).collect(...),
or if restaurant validation is required keep the current code but
document/accept the extra query; update getMenus accordingly and ensure
references to restaurantRepository.findById,
menuRepository.findAllByRestaurantId, and MenuRes::from are the points of
change.
Comment on lines
+46
to
+60
| for (CreateOrderReq.OrderItemReq itemReq : req.getItems()) { | ||
| Menu menu = menuRepository.findById(itemReq.getMenuId()) | ||
| .orElseThrow(() -> new MenuException(ErrorStatus.MENU_NOT_FOUND)); | ||
| if (!menu.getIsAvailable()) { | ||
| throw new MenuException(ErrorStatus.MENU_NOT_AVAILABLE); | ||
| } | ||
| int itemTotal = menu.getPrice() * itemReq.getQuantity(); | ||
| totalPrice += itemTotal; | ||
|
|
||
| orderItems.add(OrderItem.builder() | ||
| .menu(menu) | ||
| .quantity(itemReq.getQuantity()) | ||
| .unitPrice(menu.getPrice()) | ||
| .build()); | ||
| } |
There was a problem hiding this comment.
메뉴 조회 시 N+1 문제가 발생합니다.
반복문 내에서 각 메뉴를 개별적으로 조회하고 있어, 주문 항목 수만큼 쿼리가 발생하는 N+1 문제가 있습니다. 성능 개선을 위해 메뉴 ID 목록을 미리 수집하여 한 번의 쿼리로 조회하는 것을 권장합니다.
🔧 제안하는 개선 방법
MenuRepository에 IN 쿼리 메서드를 추가하고 일괄 조회:
// MenuRepository.java에 추가
List<Menu> findAllByIdIn(List<Long> ids); // OrderService.java
+ // 메뉴 ID 목록 수집
+ List<Long> menuIds = req.getItems().stream()
+ .map(CreateOrderReq.OrderItemReq::getMenuId)
+ .collect(Collectors.toList());
+
+ // 한 번에 조회
+ List<Menu> menus = menuRepository.findAllByIdIn(menuIds);
+ Map<Long, Menu> menuMap = menus.stream()
+ .collect(Collectors.toMap(Menu::getId, m -> m));
+
List<OrderItem> orderItems = new ArrayList<>();
int totalPrice = 0;
for (CreateOrderReq.OrderItemReq itemReq : req.getItems()) {
- Menu menu = menuRepository.findById(itemReq.getMenuId())
- .orElseThrow(() -> new MenuException(ErrorStatus.MENU_NOT_FOUND));
+ Menu menu = menuMap.get(itemReq.getMenuId());
+ if (menu == null) {
+ throw new MenuException(ErrorStatus.MENU_NOT_FOUND);
+ }
if (!menu.getIsAvailable()) {
throw new MenuException(ErrorStatus.MENU_NOT_AVAILABLE);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/service/OrderService.java` around lines 46 -
60, The loop in create order uses menuRepository.findById per item causing an
N+1 query; instead collect all menu IDs from req.getItems(), add a repository
method like findAllByIdIn(List<Long> ids) on MenuRepository, fetch all menus
once, map them by id, then iterate req.getItems() using the map to look up each
Menu (throw MenuException with ErrorStatus.MENU_NOT_FOUND if missing and
ErrorStatus.MENU_NOT_AVAILABLE if menu.getIsAvailable() is false), compute
itemTotal using menu.getPrice() * itemReq.getQuantity(), accumulate totalPrice,
and build OrderItem via OrderItem.builder() into orderItems as before.
Comment on lines
+73
to
+76
| // 연관관계 세팅 | ||
| for (OrderItem item : orderItems) { | ||
| order.getOrderItems().add(item); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# OrderItem의 양방향 연관관계 확인
rg -nP '@ManyToOne.*Order|private.*Order.*order' --type=java -A 2 -B 2 -g '**/domain/orderitem/OrderItem.java'Repository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 552
🏁 Script executed:
cat -n src/main/java/com/kuit/baemin/domain/order/Order.java | head -100Repository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 1979
🏁 Script executed:
rg -nP 'class Order|@OneToMany|addOrderItem' --type=java src/main/java/com/kuit/baemin/domain/order/Order.java -A 3Repository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 323
🏁 Script executed:
cat -n src/main/java/com/kuit/baemin/service/OrderService.java | sed -n '65,85p'Repository: Konkuk-KUIT/KUIT7_REST-API
Length of output: 657
양방향 연관관계 설정을 확인하세요.
OrderItem이 Order에 대한 @ManyToOne 참조를 가지는 양방향 연관관계로 정의되어 있습니다. 현재 코드에서는 order.getOrderItems().add(item) 만 수행하고 있으나, 양쪽 모두를 설정해야 일관성이 유지됩니다. Order 엔티티에 연관관계 편의 메서드를 추가하여 다음과 같이 리팩토링할 것을 권장합니다:
// Order.java
public void addOrderItem(OrderItem orderItem) {
this.orderItems.add(orderItem);
orderItem.setOrder(this);
}그 후 서비스 코드를 다음과 같이 수정하세요:
// 연관관계 세팅
for (OrderItem item : orderItems) {
order.addOrderItem(item);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/service/OrderService.java` around lines 73 -
76, The current loop only mutates the Order side
(order.getOrderItems().add(item)) and breaks the bidirectional contract with
OrderItem, so add a convenience mutator on Order (e.g., addOrderItem(OrderItem
orderItem)) that both adds to this.orderItems and calls
orderItem.setOrder(this), then replace the loop in OrderService to call
order.addOrderItem(item) for each OrderItem to ensure both sides (Order and
OrderItem) are kept consistent.
Comment on lines
+27
to
+33
| public ReviewRes createReview(CreateReviewReq req) { | ||
| Member member = memberRepository.findById(req.getMemberId()) | ||
| .orElseThrow(() -> new MemberException(ErrorStatus.MEMBER_NOT_FOUND)); | ||
| Order order = orderRepository.findById(req.getOrderId()) | ||
| .orElseThrow(() -> new OrderException(ErrorStatus.ORDER_NOT_FOUND)); | ||
| Restaurant restaurant = order.getRestaurant(); | ||
|
|
There was a problem hiding this comment.
주문-회원 소유권 검증이 없어 타인 주문으로 리뷰를 작성할 수 있습니다.
memberId와 orderId를 각각 존재 여부만 확인하고 있어, 주문 소유자 검증 없이 리뷰 생성이 가능합니다. order의 소유 회원과 req.getMemberId() 일치 여부를 반드시 검증해 주세요.
권장 수정 예시
Order order = orderRepository.findById(req.getOrderId())
.orElseThrow(() -> new OrderException(ErrorStatus.ORDER_NOT_FOUND));
+ if (!order.getMember().getId().equals(req.getMemberId())) {
+ throw new ReviewException(ErrorStatus.REVIEW_UNAUTHORIZED);
+ }
Restaurant restaurant = order.getRestaurant();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/service/ReviewService.java` around lines 27 -
33, The createReview method currently only checks existence of Member and Order
but does not verify ownership, allowing reviews for others' orders; after
retrieving member (memberRepository.findById(...)) and order
(orderRepository.findById(...)), add a check that order.getMember().getId() (or
order.getMember()) equals req.getMemberId() (or member) and if not throw a
suitable exception (e.g., new OrderException(ErrorStatus.ORDER_NOT_OWNER) or
reuse OrderException/ErrorStatus.ORDER_NOT_FOUND with a clearer status),
preventing creation when the requester is not the order owner; update unit tests
for createReview to cover mismatched memberId/orderId case.
Comment on lines
+29
to
+31
| .orElseThrow(() -> new MemberException(ErrorStatus.MEMBER_NOT_FOUND)); | ||
| Order order = orderRepository.findById(req.getOrderId()) | ||
| .orElseThrow(() -> new OrderException(ErrorStatus.ORDER_NOT_FOUND)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
예외 처리 패턴을 GeneralException(ErrorStatus.XXX)로 통일해 주세요.
현재 서비스에서 도메인별 커스텀 예외를 직접 던지고 있어, 가이드의 예외 처리 패턴과 불일치합니다.
As per coding guidelines, "예외 처리 시 GeneralException(ErrorStatus.XXX) 패턴 사용 여부".
Also applies to: 47-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/kuit/baemin/service/ReviewService.java` around lines 29 -
31, Replace domain-specific exceptions with the general exception pattern: in
ReviewService where MemberException and OrderException are thrown (e.g., the
expressions creating new MemberException(ErrorStatus.MEMBER_NOT_FOUND) and new
OrderException(ErrorStatus.ORDER_NOT_FOUND)), throw new
GeneralException(ErrorStatus.MEMBER_NOT_FOUND) and new
GeneralException(ErrorStatus.ORDER_NOT_FOUND) instead; do the same for the other
occurrences noted (lines around 47-49) so all service-level throws use
GeneralException(ErrorStatus.XXX).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
구현한 API 목록
/members/members/login/members/{memberId}/restaurants/restaurants/restaurants/{restaurantId}/restaurants/{restaurantId}/menus/restaurants/{restaurantId}/menus/members/{memberId}/addresses/members/{memberId}/addresses/orders/reviews/reviews/{reviewId}Summary by CodeRabbit
New Features