7주차 미션 / 서버 1조 정연석 - #8
Open
yeon-seokk wants to merge 1 commit into
Open
yeon-seokk wants to merge 1 commit into
yeon-seokk wants to merge 1 commit into
Conversation
Walkthrough이 PR은 배달 앱 REST API에 주소 관리, 주문 생성, 메뉴/레스토랑 조회 기능을 추가하고, 기존 회원 관리를 /members에서 /users로 마이그레이션하며 Database 테이블 매핑을 업데이트합니다. Changes주소, 주문, 메뉴, 레스토랑 및 회원 기능 통합
🎯 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: 24
♻️ Duplicate comments (1)
src/main/java/com/kuit/baemin/dto/response/DefaultAddressRes.java (1)
17-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick win지연 로딩 접근에 주의하세요.
AddressRes.java의 Line 24와 동일하게,address.getMember().getId()호출 시LazyInitializationException이 발생할 수 있습니다.AddressService에서Member를 함께 로드해야 합니다.🤖 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/response/DefaultAddressRes.java` at line 17, DefaultAddressRes 생성 시 address.getMember().getId()를 직접 호출하면 지연 로딩으로 LazyInitializationException이 발생할 수 있으니, AddressService에서 DefaultAddressRes를 생성하기 전에 Member를 함께 로드하도록 수정하세요; 구체적으로는 AddressService 내에서 Address 조회 시 Member를 fetch join 하거나 리포지토리에서 멤버를 함께 반환하도록 쿼리를 변경하고, 그 로드된 엔티티의 getMember().getId()를 사용해 DefaultAddressRes를 생성하도록 바꿔주세요 (참조 메서드: DefaultAddressRes, address.getMember().getId(), AddressService, Member).
🤖 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/RestaurantController.java`:
- Line 20: The controller currently accepts a Pageable (parameter named
pageable) without enforcing a maximum page size, allowing clients to request
arbitrarily large pages; update RestaurantController to enforce a max page size
by either (A) reading spring.data.web.pageable.max-page-size from configuration
and applying it when resolving Pageable, or (B) validating and capping the
incoming Pageable.size in the controller/service method (e.g., in the method
that accepts Pageable pageable, check pageable.getPageSize() and replace it with
Math.min(requestedSize, MAX_PAGE_SIZE) before passing to the
repository/service). Use a single constant or config-backed value (e.g.,
MAX_PAGE_SIZE) and reference the pageable parameter and the controller method
handling restaurant listing to implement the cap.
In `@src/main/java/com/kuit/baemin/domain/address/Address.java`:
- Around line 57-58: In class Address, remove the stray space between the
"this." qualifier and the field names so assignments use "this.latitude =
latitude" and "this.longitude = longitude"; update the constructor or any
methods in Address that contain "this. latitude" / "this. longitude" to remove
the extra whitespace so the field references are correct and consistent.
- Line 56: Whitespace around operators in the Address class assignment is
incorrect; update the expression in the Address constructor/initializer (the
line using isDefault !=null ? isDefault:false) to include spaces around
operators so it reads: isDefault != null ? isDefault : false; ensuring proper
spacing around !=, ?: and the colon.
In `@src/main/java/com/kuit/baemin/domain/member/Member.java`:
- Around line 43-45: In Member.java the nickname update branch incorrectly
checks name != null, causing nickname updates to be skipped or overwritten;
change the conditional to check nickname != null before assigning this.nickname
= nickname (inside the method that updates member fields, e.g., the update/patch
method in class Member) so nickname is only updated when a new nickname is
provided and other fields remain unaffected.
In `@src/main/java/com/kuit/baemin/domain/menu/Menu.java`:
- Around line 41-55: The Menu constructor allows null for isSoldOut despite the
field being `@Column`(nullable = false); update the Menu entity to make the
default intent explicit by applying `@Builder.Default` to the isSoldOut field (and
initialize it to false) and remove nullable handling from the constructor body
(keep constructor parameter but rely on the field default), so that the Menu
class, its constructor, and the isSoldOut field consistently use a non-null
default value; refer to the Menu class, the Menu(...) builder-enabled
constructor, and the isSoldOut field when making this change.
In `@src/main/java/com/kuit/baemin/domain/Restaurant/Restaurant.java`:
- Line 15: You changed the JPA mapping on the Restaurant entity (`@Table`(name =
"stores")) which does not migrate existing data; add a DB migration that either
renames the existing restaurants table to stores or copies data into a new
stores table and preserves constraints/indices. Specifically, create a migration
script (Flyway/Liquibase) that runs before app startup to perform ALTER TABLE
RENAME FROM restaurants TO stores or INSERT-SELECT with schema/constraint
creation, and include rollback steps; reference the Restaurant entity and its
`@Table` mapping so the schema and constraints match the entity fields.
- Around line 37-41: Add non-negative constraints to the Restaurant money
fields: annotate the fields minOrderPrice and deliveryFee with
javax.validation.constraints.@Min(0) for runtime/DTO validation and add a
DB-level CHECK to guarantee integrity (either via Hibernate's `@Check` at the
Restaurant class level or via Column(columnDefinition) with "check" expression)
ensuring min_order_price >= 0 and delivery_fee >= 0; update any service-side
setters/constructors that accept these values to validate/throw on negative
input so domain invariants are enforced before persistence.
In `@src/main/java/com/kuit/baemin/dto/request/UpdateAddressReq.java`:
- Line 8: In UpdateAddressReq, remove the leading space in the `@Size` validation
message string so it reads message="주소 이름은 50자 이하여야 합니다." (update the `@Size`
annotation on the field in class UpdateAddressReq to eliminate the unnecessary
whitespace before '주소').
In `@src/main/java/com/kuit/baemin/dto/request/UpdateMemberReq.java`:
- Around line 10-14: The DTO UpdateMemberReq currently allows empty or
whitespace-only values for fields like name and nickname (and the other fields
around lines 22-24); update those fields to reject blank input by adding
appropriate validation such as `@NotBlank` (or `@Size`(min=1) combined with a
trim/isBlank check) on String fields, e.g., annotate name and nickname in
UpdateMemberReq with `@NotBlank` (import javax.validation.constraints.NotBlank) or
use `@Size`(min=1) plus a custom isBlank guard so blank/whitespace-only strings
are invalid; ensure any other request fields mentioned (around lines 22-24)
receive the same treatment.
In `@src/main/java/com/kuit/baemin/dto/response/AddressRes.java`:
- Line 24: The DTO mapping in AddressRes uses address.getMember().getId() which
can trigger LazyInitializationException because Member is LAZY and the
persistence session may be closed; update the repository query that loads
Address (e.g., the AddressRepository method used by the service such as
findById/fetchAddressesByMemberId) to eagerly fetch Member—either add an
`@EntityGraph`(attributePaths = "member") on that repository method or rewrite its
`@Query` to use join fetch (join fetch a.member) so Member is loaded before the
DTO mapping in AddressRes; alternatively, adjust the service to call a
repository projection/DTO query that selects the member id directly if you
prefer not to change fetch strategy.
In `@src/main/java/com/kuit/baemin/dto/response/CreateOrderRes.java`:
- Around line 21-31: The static CreateOrderRes.from(OrderEntity order) eagerly
dereferences lazy associations (order.getMember().getId(),
order.getRestaurant().getId(), order.getAddress().getId()) which will throw
LazyInitializationException when called outside a transaction; to fix, ensure
callers load required associations in the service layer (e.g., fetch member,
restaurant, address IDs or join-fetch OrderEntity) before calling
CreateOrderRes.from, or add an overloaded factory that accepts the
already-loaded IDs (e.g., from(OrderEntity, Long memberId, Long storeId, Long
addressId)) so CreateOrderRes.from and the service methods (where
OrderService.createOrder is) do not access lazy proxies outside transactional
context.
In `@src/main/java/com/kuit/baemin/exception/errorcode/ErrorStatus.java`:
- Line 25: Update the comment headers for the address and store sections to
match the existing decorative format used elsewhere (e.g., change plain "// 주소"
and "// 가게" to the same style as "// ── 회원 ──") in ErrorStatus.java so all
section headers are consistent; locate the address and store section comments
and replace them with the decorative comment pattern used for other sections.
- Around line 26-28: The two enum entries INVALID_ADDRESS and
ADDRESS_EMPTY_UPDATE_VALUE in ErrorStatus both use the same error code string
"ADDRESS400"; change ADDRESS_EMPTY_UPDATE_VALUE to a unique error code (e.g.,
"ADDRESS401" or another unused ADDRESSxxx value) so every ErrorStatus has a
distinct code, and update any references/tests that assert the old code to use
the new value; locate these constants in the ErrorStatus enum (INVALID_ADDRESS,
ADDRESS_EMPTY_UPDATE_VALUE) and modify only the code string for
ADDRESS_EMPTY_UPDATE_VALUE.
- Line 35: ErrorStatus 열거형에서 ORDER_MIN_PRICE_NOT_MET 상수가 가게(매장) 섹션에 잘못 위치해 있으니,
ErrorStatus(enum) 안에 별도의 주문(ORDER) 섹션을 만들어
ORDER_MIN_PRICE_NOT_MET(HttpStatus.BAD_REQUEST, "ORDER400", "최소 주문 금액을 충족하지
못했습니다.") 항목을 해당 주문 섹션으로 이동하고 다른 주문 관련 상수(예: ORDER_* 또는 PAYMENT_* 관련 항목)가 있다면 함께
그룹화하여 섹션 주석 또는 구분자(예: // ORDER errors)로 구분하세요.
In `@src/main/java/com/kuit/baemin/repository/MenuRepositroy.java`:
- Line 9: Interface name has a typo: rename the interface MenuRepositroy to
MenuRepository (and update the filename accordingly) so the declaration reads
interface MenuRepository extends JpaRepository<Menu, Long>; then update all
references/usages (e.g., the field or injection point in MenuService) to use the
corrected type name MenuRepository to keep symbols consistent.
In `@src/main/java/com/kuit/baemin/service/AddressService.java`:
- Line 33: Replace the service-layer-specific MemberException usages with the
standardized GeneralException(ErrorStatus.XXX) pattern: locate the Member lookup
calls in AddressService (the lines using .orElseThrow(()-> new
MemberException(ErrorStatus.MEMBER_NOT_FOUND)) and the similar occurrence around
Line 49) and change them to throw new
GeneralException(ErrorStatus.MEMBER_NOT_FOUND) so the service follows the
project's exception convention; ensure imports are updated if needed.
- Around line 31-43: The service currently writes isDefault=true directly in
AddressService.createAddress (and similar update paths) which can create
multiple defaults under concurrency; wrap the create/update flows in a single
`@Transactional` method in AddressService and, when req.getIsDefault() is true,
perform an atomic DB update via addressRepository (e.g. a JPQL/SQL bulk update
method like clearDefaultForMember(member.getId()) that issues "UPDATE address
SET is_default=false WHERE member_id=:id") before saving the new Address so the
unset and insert happen in one transaction; additionally add a DB-level
uniqueness constraint/index (unique partial index on (member_id) WHERE
is_default = true) on the Address table and catch
DataIntegrityViolationException from addressRepository.save(...) to translate
into a clear domain exception so races still produce a handled error.
In `@src/main/java/com/kuit/baemin/service/MemberService.java`:
- Around line 78-85: In updateMember, add the same deleted-state guard used in
deleteMember: after retrieving Member via memberRepository.findById(...) and
before calling member.updateInfo(...), check member.isDeleted() and if true
throw the same MemberException (e.g., new MemberException(MEMBER_NOT_FOUND) or
the project's designated deleted-member error); this prevents modifying
soft-deleted members and keeps behavior consistent with deleteMember;
updateMember, member.isDeleted(), MemberException, MEMBER_NOT_FOUND,
member.updateInfo(...) and MemberRes.from(...) are the relevant symbols to
change.
- Around line 74-95: The class-level `@Transactional`(readOnly = true) causes
updates in MemberService.updateMember and MemberService.deleteMember not to be
persisted; annotate both updateMember(Long userId, UpdateMemberReq req) and
deleteMember(Long userId) with `@Transactional` (no readOnly) so they run in a
read-write transaction and ensure the import for
org.springframework.transaction.annotation.Transactional is added.
In `@src/main/java/com/kuit/baemin/service/MenuService.java`:
- Line 23: The field name retaurantRepository is misspelled; rename it to
restaurantRepository across the class to fix references and avoid compilation
errors: update the private final field declaration retaurantRepository to
restaurantRepository and adjust all usages (constructor parameters, assignments,
methods) that reference retaurantRepository (e.g., in MenuService) so they use
restaurantRepository consistently.
In `@src/main/java/com/kuit/baemin/service/OrderService.java`:
- Around line 60-63: The current minimum-order validation in OrderService uses
totalPrice = restaurant.getMinOrderPrice() + deliveryFee and then checks
totalPrice < restaurant.getMinOrderPrice(), which always fails to detect
violations; either remove or comment out this meaningless check now, and when
order items are added implement proper validation by summing item prices (e.g.,
sum of items' unitPrice * quantity) + deliveryFee and comparing that sum against
restaurant.getMinOrderPrice(), throwing
GeneralException(ErrorStatus.ORDER_MIN_PRICE_NOT_MET) when the computed total is
below the minimum.
- Around line 76-78: The generateOrderNumber() method currently returns "ORD-" +
System.currentTimeMillis(), which may produce duplicate order IDs under high
concurrency; update generateOrderNumber() to produce collision-resistant IDs
(e.g., use a UUID-based value like "ORD-"+UUID.randomUUID().toString(), or
combine timestamp with a secure random suffix, or delegate to a DB sequence) and
ensure the chosen approach is used wherever order numbers are created and
persisted so uniqueness is guaranteed.
In `@src/main/resources/application.yml`:
- Line 10: The application.yml currently sets spring.jpa.hibernate.ddl-auto to
"update", which can miss certain schema changes and is unsafe for production;
either revert to "create" while actively developing entity models or implement a
proper migration strategy (introduce Flyway or Liquibase and remove/replace the
ddl-auto setting), and ensure production uses "validate" or "none"; update the
configuration and add a short comment/documentation near the ddl-auto entry to
indicate the chosen mode and reasoning so reviewers see whether the project is
using automatic schema updates (ddl-auto in application.yml) or managed
migrations.
- Line 10: 현재 application.yml의 설정 키 ddl-auto 값은 'update'인데 주석에는 "개발 중에는
create"라고 쓰여 있어 불일치합니다; 수정할 때는 ddl-auto 값을 그대로 유지하려면 주석을 '개발 중에는 update(필요 시
create로 변경)' 또는 유사한 정확한 설명으로 변경하거나, 주석대로 개발용이라면 ddl-auto 값을 'create'로 변경하세요; 참조할
식별자: 설정 키 ddl-auto 및 현재 값 'update' (또는 대체값 'create').
---
Duplicate comments:
In `@src/main/java/com/kuit/baemin/dto/response/DefaultAddressRes.java`:
- Line 17: DefaultAddressRes 생성 시 address.getMember().getId()를 직접 호출하면 지연 로딩으로
LazyInitializationException이 발생할 수 있으니, AddressService에서 DefaultAddressRes를 생성하기
전에 Member를 함께 로드하도록 수정하세요; 구체적으로는 AddressService 내에서 Address 조회 시 Member를 fetch
join 하거나 리포지토리에서 멤버를 함께 반환하도록 쿼리를 변경하고, 그 로드된 엔티티의 getMember().getId()를 사용해
DefaultAddressRes를 생성하도록 바꿔주세요 (참조 메서드: DefaultAddressRes,
address.getMember().getId(), AddressService, Member).
🪄 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: d6c7c30d-e77c-46ce-8248-c5d149cbb957
📒 Files selected for processing (40)
src/main/java/com/kuit/baemin/controller/AddressController.javasrc/main/java/com/kuit/baemin/controller/MemberController.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/domain/Restaurant/Restaurant.javasrc/main/java/com/kuit/baemin/domain/address/Address.javasrc/main/java/com/kuit/baemin/domain/address/AddressStatus.javasrc/main/java/com/kuit/baemin/domain/member/Member.javasrc/main/java/com/kuit/baemin/domain/menu/Menu.javasrc/main/java/com/kuit/baemin/domain/menu/MenuStatus.javasrc/main/java/com/kuit/baemin/domain/order/OrderEntity.javasrc/main/java/com/kuit/baemin/domain/order/OrderStatus.javasrc/main/java/com/kuit/baemin/dto/request/CreateAddressReq.javasrc/main/java/com/kuit/baemin/dto/request/CreateOrderReq.javasrc/main/java/com/kuit/baemin/dto/request/SignUpReq.javasrc/main/java/com/kuit/baemin/dto/request/UpdateAddressReq.javasrc/main/java/com/kuit/baemin/dto/request/UpdateMemberReq.javasrc/main/java/com/kuit/baemin/dto/response/AddressListRes.javasrc/main/java/com/kuit/baemin/dto/response/AddressRes.javasrc/main/java/com/kuit/baemin/dto/response/CreateOrderRes.javasrc/main/java/com/kuit/baemin/dto/response/DefaultAddressRes.javasrc/main/java/com/kuit/baemin/dto/response/DeleteAddressRes.javasrc/main/java/com/kuit/baemin/dto/response/DeleteMemberRes.javasrc/main/java/com/kuit/baemin/dto/response/MemberRes.javasrc/main/java/com/kuit/baemin/dto/response/MenuListRes.javasrc/main/java/com/kuit/baemin/dto/response/MenuRes.javasrc/main/java/com/kuit/baemin/dto/response/RestaurantListRes.javasrc/main/java/com/kuit/baemin/dto/response/RestaurantRes.javasrc/main/java/com/kuit/baemin/exception/errorcode/ErrorStatus.javasrc/main/java/com/kuit/baemin/repository/AddressRepository.javasrc/main/java/com/kuit/baemin/repository/MenuRepositroy.javasrc/main/java/com/kuit/baemin/repository/OrderRepository.javasrc/main/java/com/kuit/baemin/repository/RestaurantRepository.javasrc/main/java/com/kuit/baemin/service/AddressService.javasrc/main/java/com/kuit/baemin/service/MemberService.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/resources/application.yml
|
|
||
| @GetMapping("/stores") | ||
| public ApiResponse<RestaurantListRes> getStores( | ||
| @PageableDefault(size=10) Pageable pageable |
There was a problem hiding this comment.
페이지 크기 상한을 강제해 주세요.
현재는 클라이언트가 매우 큰 size를 전달할 수 있어 대량 조회로 성능 저하/타임아웃 위험이 있습니다. spring.data.web.pageable.max-page-size 설정 또는 서비스 단 검증으로 상한을 두는 것이 필요합니다.
🤖 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/RestaurantController.java` at line
20, The controller currently accepts a Pageable (parameter named pageable)
without enforcing a maximum page size, allowing clients to request arbitrarily
large pages; update RestaurantController to enforce a max page size by either
(A) reading spring.data.web.pageable.max-page-size from configuration and
applying it when resolving Pageable, or (B) validating and capping the incoming
Pageable.size in the controller/service method (e.g., in the method that accepts
Pageable pageable, check pageable.getPageSize() and replace it with
Math.min(requestedSize, MAX_PAGE_SIZE) before passing to the
repository/service). Use a single constant or config-backed value (e.g.,
MAX_PAGE_SIZE) and reference the pageable parameter and the controller method
handling restaurant listing to implement the cap.
| ){ | ||
| this.addressName = addressName; | ||
| this.address = address; | ||
| this.isDefault = isDefault !=null ? isDefault:false; |
There was a problem hiding this comment.
띄어쓰기를 수정하세요.
연산자 양쪽에 공백이 필요합니다.
✏️ 수정 제안
- this.isDefault = isDefault !=null ? isDefault:false;
+ this.isDefault = isDefault != null ? isDefault : false;📝 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.
Suggested change
| this.isDefault = isDefault !=null ? isDefault:false; | |
| this.isDefault = isDefault != null ? 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/domain/address/Address.java` at line 56,
Whitespace around operators in the Address class assignment is incorrect; update
the expression in the Address constructor/initializer (the line using isDefault
!=null ? isDefault:false) to include spaces around operators so it reads:
isDefault != null ? isDefault : false; ensuring proper spacing around !=, ?: and
the colon.
Comment on lines
+57
to
+58
| this. latitude = latitude; | ||
| this. longitude = longitude; |
There was a problem hiding this comment.
불필요한 공백을 제거하세요.
this.와 필드명 사이에 공백이 있습니다.
✏️ 수정 제안
- this. latitude = latitude;
- this. longitude = longitude;
+ this.latitude = latitude;
+ this.longitude = longitude;📝 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.
Suggested change
| this. latitude = latitude; | |
| this. longitude = longitude; | |
| this.latitude = latitude; | |
| this.longitude = longitude; |
🤖 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/address/Address.java` around lines 57 -
58, In class Address, remove the stray space between the "this." qualifier and
the field names so assignments use "this.latitude = latitude" and
"this.longitude = longitude"; update the constructor or any methods in Address
that contain "this. latitude" / "this. longitude" to remove the extra whitespace
so the field references are correct and consistent.
Comment on lines
+43
to
+45
| if(name != null){ | ||
| this.nickname = nickname; | ||
| } |
There was a problem hiding this comment.
닉네임 갱신 조건이 잘못되어 부분 수정이 깨집니다.
nickname 갱신 분기에서 name != null을 검사하고 있어, 닉네임만 수정 요청 시 반영되지 않고 이름만 수정할 때 닉네임이 null로 덮일 수 있습니다.
수정 제안
- if(name != null){
+ if(nickname != null){
this.nickname = nickname;
}📝 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.
Suggested change
| if(name != null){ | |
| this.nickname = nickname; | |
| } | |
| if(nickname != null){ | |
| this.nickname = nickname; | |
| } |
🤖 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/member/Member.java` around lines 43 -
45, In Member.java the nickname update branch incorrectly checks name != null,
causing nickname updates to be skipped or overwritten; change the conditional to
check nickname != null before assigning this.nickname = nickname (inside the
method that updates member fields, e.g., the update/patch method in class
Member) so nickname is only updated when a new nickname is provided and other
fields remain unaffected.
Comment on lines
+41
to
+55
| @Builder | ||
| public Menu( | ||
| String name, | ||
| Integer price, | ||
| String description, | ||
| Boolean isSoldOut, | ||
| Restaurant restaurant | ||
| ) { | ||
| this.name = name; | ||
| this.price = price; | ||
| this.description = description; | ||
| this.isSoldOut = isSoldOut != null ? isSoldOut : false; | ||
| this.restaurant = restaurant; | ||
| this.status = MenuStatus.ACTIVE; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
isSoldOut 기본값 처리 개선 제안
isSoldOut이 @Column(nullable = false)로 선언되었지만 생성자에서는 null을 허용하고 있습니다. 현재 로직은 동작하지만, @Builder.Default 어노테이션을 사용하면 의도가 더 명확해집니다.
♻️ 개선 제안
`@Column`(name = "is_sold_out", nullable = false)
+ `@Builder.Default`
private Boolean isSoldOut;
...
`@Builder`
public Menu(
String name,
Integer price,
String description,
Boolean isSoldOut,
Restaurant restaurant
) {
this.name = name;
this.price = price;
this.description = description;
- this.isSoldOut = isSoldOut != null ? isSoldOut : false;
+ this.isSoldOut = isSoldOut;
this.restaurant = restaurant;
this.status = MenuStatus.ACTIVE;
}🤖 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/menu/Menu.java` around lines 41 - 55,
The Menu constructor allows null for isSoldOut despite the field being
`@Column`(nullable = false); update the Menu entity to make the default intent
explicit by applying `@Builder.Default` to the isSoldOut field (and initialize it
to false) and remove nullable handling from the constructor body (keep
constructor parameter but rely on the field default), so that the Menu class,
its constructor, and the isSoldOut field consistently use a non-null default
value; refer to the Menu class, the Menu(...) builder-enabled constructor, and
the isSoldOut field when making this change.
Comment on lines
+78
to
+85
| Member member = memberRepository.findById(userId) | ||
| .orElseThrow(() -> new MemberException(MEMBER_NOT_FOUND)); | ||
| member.updateInfo( | ||
| req.getName(), | ||
| req.getNickname(), | ||
| req.getPhone() | ||
| ); | ||
| return MemberRes.from(member); |
There was a problem hiding this comment.
삭제된 회원에 대한 수정이 허용되고 있습니다.
updateMember에서 삭제 상태를 검사하지 않아 탈퇴 회원 정보가 다시 변경될 수 있습니다. deleteMember와 동일하게 member.isDeleted() 가드를 추가해주세요.
수정 제안
Member member = memberRepository.findById(userId)
.orElseThrow(() -> new MemberException(MEMBER_NOT_FOUND));
+ if (member.isDeleted()) {
+ throw new MemberException(ErrorStatus.MEMBER_ALREADY_DELETED);
+ }
member.updateInfo(🤖 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/MemberService.java` around lines 78 -
85, In updateMember, add the same deleted-state guard used in deleteMember:
after retrieving Member via memberRepository.findById(...) and before calling
member.updateInfo(...), check member.isDeleted() and if true throw the same
MemberException (e.g., new MemberException(MEMBER_NOT_FOUND) or the project's
designated deleted-member error); this prevents modifying soft-deleted members
and keeps behavior consistent with deleteMember; updateMember,
member.isDeleted(), MemberException, MEMBER_NOT_FOUND, member.updateInfo(...)
and MemberRes.from(...) are the relevant symbols to change.
| @Transactional(readOnly = true) | ||
| public class MenuService { | ||
| private final MenuRepositroy menuRepositroy; | ||
| private final RestaurantRepository retaurantRepository; |
There was a problem hiding this comment.
필드명 오타 수정 필요
필드명이 retaurantRepository로 철자가 잘못되었습니다. restaurantRepository로 수정해야 합니다.
🐛 수정 제안
- private final RestaurantRepository retaurantRepository;
+ private final RestaurantRepository restaurantRepository;Line 26에서도 동일하게 수정:
- Restaurant restaurant = retaurantRepository.findById(storeId)
+ Restaurant restaurant = restaurantRepository.findById(storeId)📝 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.
Suggested change
| private final RestaurantRepository retaurantRepository; | |
| private final RestaurantRepository restaurantRepository; |
🤖 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` at line 23, The field
name retaurantRepository is misspelled; rename it to restaurantRepository across
the class to fix references and avoid compilation errors: update the private
final field declaration retaurantRepository to restaurantRepository and adjust
all usages (constructor parameters, assignments, methods) that reference
retaurantRepository (e.g., in MenuService) so they use restaurantRepository
consistently.
Comment on lines
+60
to
+63
| Integer totalPrice = restaurant.getMinOrderPrice() + deliveryFee; | ||
| if (totalPrice < restaurant.getMinOrderPrice()) { | ||
| throw new GeneralException(ErrorStatus.ORDER_MIN_PRICE_NOT_MET); | ||
| } |
There was a problem hiding this comment.
최소 주문 금액 검증 로직이 항상 통과합니다.
Line 60에서 totalPrice = restaurant.getMinOrderPrice() + deliveryFee로 계산한 후, Line 61에서 totalPrice < restaurant.getMinOrderPrice()를 검증하고 있습니다. deliveryFee가 음수가 아닌 한 이 조건은 항상 false이므로 검증이 무의미합니다.
주석(Line 54-59)에 명시된 것처럼 현재는 실제 주문 메뉴 항목이 없어 임시로 최소 주문 금액을 사용하고 있습니다. 향후 주문 항목(items)을 추가할 때 실제 메뉴 금액 합계를 계산하여 검증하도록 수정해야 합니다.
🔧 수정 제안
현재 명세의 한계로 인한 임시 구현이므로, 검증 로직을 제거하거나 주석으로 대체하는 것을 권장합니다:
- Integer totalPrice = restaurant.getMinOrderPrice() + deliveryFee;
- if (totalPrice < restaurant.getMinOrderPrice()) {
- throw new GeneralException(ErrorStatus.ORDER_MIN_PRICE_NOT_MET);
- }
+ // TODO: 주문 항목(items) 추가 후 실제 메뉴 금액 합계로 최소 주문 금액 검증
+ Integer totalPrice = restaurant.getMinOrderPrice() + deliveryFee;📝 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.
Suggested change
| Integer totalPrice = restaurant.getMinOrderPrice() + deliveryFee; | |
| if (totalPrice < restaurant.getMinOrderPrice()) { | |
| throw new GeneralException(ErrorStatus.ORDER_MIN_PRICE_NOT_MET); | |
| } | |
| // TODO: 주문 항목(items) 추가 후 실제 메뉴 금액 합계로 최소 주문 금액 검증 | |
| Integer totalPrice = restaurant.getMinOrderPrice() + deliveryFee; |
🤖 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 60 -
63, The current minimum-order validation in OrderService uses totalPrice =
restaurant.getMinOrderPrice() + deliveryFee and then checks totalPrice <
restaurant.getMinOrderPrice(), which always fails to detect violations; either
remove or comment out this meaningless check now, and when order items are added
implement proper validation by summing item prices (e.g., sum of items'
unitPrice * quantity) + deliveryFee and comparing that sum against
restaurant.getMinOrderPrice(), throwing
GeneralException(ErrorStatus.ORDER_MIN_PRICE_NOT_MET) when the computed total is
below the minimum.
Comment on lines
+76
to
+78
| private String generateOrderNumber() { | ||
| return "ORD-" + System.currentTimeMillis(); | ||
| } |
There was a problem hiding this comment.
주문번호 생성 시 중복 가능성이 있습니다.
System.currentTimeMillis()는 밀리초 단위 타임스탬프를 반환하므로, 동일한 밀리초 내에 여러 주문이 발생하면 주문번호가 중복될 수 있습니다. 특히 부하가 높은 환경에서는 충돌 가능성이 높습니다.
💡 개선 제안
다음 중 하나의 방법을 권장합니다:
방법 1: UUID 사용
private String generateOrderNumber() {
- return "ORD-" + System.currentTimeMillis();
+ return "ORD-" + UUID.randomUUID().toString();
}방법 2: 타임스탬프 + 랜덤값 조합
private String generateOrderNumber() {
- return "ORD-" + System.currentTimeMillis();
+ return "ORD-" + System.currentTimeMillis() + "-" + ThreadLocalRandom.current().nextInt(1000, 9999);
}방법 3: DB 시퀀스 활용 (가장 안전)
- 별도의 주문번호 시퀀스 테이블을 생성하여 관리
🤖 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 76 -
78, The generateOrderNumber() method currently returns "ORD-" +
System.currentTimeMillis(), which may produce duplicate order IDs under high
concurrency; update generateOrderNumber() to produce collision-resistant IDs
(e.g., use a UUID-based value like "ORD-"+UUID.randomUUID().toString(), or
combine timestamp with a secure random suffix, or delegate to a DB sequence) and
ensure the chosen approach is used wherever order numbers are created and
persisted so uniqueness is guaranteed.
| jpa: | ||
| hibernate: | ||
| ddl-auto: create # 개발 중에는 create, 이후 validate로 변경 | ||
| ddl-auto: update # 개발 중에는 create, 이후 validate로 변경 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ddl-auto 설정 변경 시 주의사항
create에서 update로 변경하면 애플리케이션 재시작 시 데이터가 보존되지만, 다음 사항을 유의해야 합니다:
update는 컬럼 이름 변경, 제약조건 변경 등 일부 스키마 변경을 제대로 처리하지 못할 수 있습니다- 개발 중 엔티티 구조를 자주 변경하는 경우,
create를 사용하거나 수동 마이그레이션(Flyway, Liquibase)을 고려하는 것이 안전합니다 - 운영 환경에서는 반드시
validate또는none을 사용해야 합니다
현재 개발 단계에서 엔티티 변경이 빈번하다면 create를 유지하거나, 안정화 단계라면 수동 마이그레이션 도구 도입을 검토해보세요.
🤖 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/resources/application.yml` at line 10, The application.yml currently
sets spring.jpa.hibernate.ddl-auto to "update", which can miss certain schema
changes and is unsafe for production; either revert to "create" while actively
developing entity models or implement a proper migration strategy (introduce
Flyway or Liquibase and remove/replace the ddl-auto setting), and ensure
production uses "validate" or "none"; update the configuration and add a short
comment/documentation near the ddl-auto entry to indicate the chosen mode and
reasoning so reviewers see whether the project is using automatic schema updates
(ddl-auto in application.yml) or managed migrations.
주석과 실제 설정값 불일치
주석에는 "개발 중에는 create"라고 명시되어 있지만, 실제 설정값은 update로 되어 있습니다. 이는 혼란을 야기할 수 있습니다.
현재 update 설정을 유지하려면 주석을 수정하거나, 주석대로 개발 중이라면 create를 사용하는 것을 고려해보세요.
🔧 제안하는 수정안 (주석 수정)
- ddl-auto: update # 개발 중에는 create, 이후 validate로 변경
+ ddl-auto: update # 개발/테스트: update, 운영: validate📝 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.
Suggested change
| ddl-auto: update # 개발 중에는 create, 이후 validate로 변경 | |
| ddl-auto: update # 개발/테스트: update, 운영: validate |
🤖 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/resources/application.yml` at line 10, 현재 application.yml의 설정 키
ddl-auto 값은 'update'인데 주석에는 "개발 중에는 create"라고 쓰여 있어 불일치합니다; 수정할 때는 ddl-auto 값을
그대로 유지하려면 주석을 '개발 중에는 update(필요 시 create로 변경)' 또는 유사한 정확한 설명으로 변경하거나, 주석대로
개발용이라면 ddl-auto 값을 'create'로 변경하세요; 참조할 식별자: 설정 키 ddl-auto 및 현재 값 'update' (또는
대체값 'create').
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 목록
/users/{userId}/users/{userId}/users/{userId}/addresses/users/{userId}/addresses/addresses/{addressId}/addresses/{addressId}/addresses/{addressId}/default/stores/stores/{storeId}/menus/ordersSummary by CodeRabbit
릴리스 노트