Open
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- 기존 Example 도메인의 전체 레이어(domain, infrastructure, interfaces) 코드 및 테스트 제거
- 불변 record 기반 Value Object 구현 - 금액 연산(plus, minus) 및 비교(isGreaterThanOrEqual) 지원 - 음수 금액 방지 검증 포함
- BrandModel Entity 및 생성 시 유효성 검증 - DIP 적용: Domain에 Repository 인터페이스 정의, Infrastructure에서 구현
- ProductModel에 재고 차감(decreaseStock) 도메인 메서드 구현 - 재고 음수 방지를 도메인 레벨에서 처리 - DIP 적용한 3-Tier Repository 구조
- 좋아요를 User-Product 관계의 별도 도메인으로 분리 - 등록/취소/조회 및 중복 좋아요 방지 도메인 규칙 포함
- PointModel에 충전(charge), 사용(use) 도메인 메서드 구현 - Money VO를 활용한 금액 연산 및 잔액 부족 시 도메인 예외 처리
- OrderModel + OrderItemModel로 주문-주문항목 구조 설계 - 총 금액 계산 등 비즈니스 로직을 도메인 객체에 캡슐화
- MemberFacade에서 Member + Point 도메인 서비스 조합 - 포인트 충전, 내 정보 조회(잔액 포함) 유스케이스 구현 - Facade 단위 테스트 작성
- BrandFacade를 통한 브랜드 등록/조회 유스케이스 구현 - Controller, ApiSpec, DTO 및 Facade 단위 테스트 작성
- ProductFacade에서 Product + Brand + Like 조합하여 상품 상세 정보 제공 - 정렬 조건(latest, price_asc, likes_desc) 지원 - Facade 단위 테스트 작성
- LikeFacade에서 회원 인증 및 상품 검증 후 좋아요 등록/취소 처리 - Facade 단위 테스트 작성
- OrderFacade에서 재고 차감 → 주문 생성 → 포인트 차감 유스케이스 조율 - Application Layer에서 도메인 서비스 조합, 핵심 로직은 Entity에 위임 - Facade 단위 테스트 작성
- 사용하지 않는 ServerWebInputException(WebFlux) 핸들러 및 관련 메서드 제거
📝 WalkthroughWalkthrough브랜드, 회원, 상품, 주문, 포인트, 좋아요 도메인 모델 및 서비스 계층 추가; Example 관련 코드 전체 제거; 다양한 API 컨트롤러와 DTO 추가 및 포괄적인 단위/통합 테스트 추가했다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrderController
participant OrderFacade
participant MemberService
participant ProductService
participant OrderService
participant PointService
Client->>OrderController: POST /api/v1/orders (loginId, password, items)
OrderController->>OrderFacade: createOrder(loginId, password, itemRequests)
OrderFacade->>MemberService: getMyInfo(loginId, password)
MemberService-->>OrderFacade: MemberModel
loop for each item
OrderFacade->>ProductService: getById(productId)
ProductService-->>OrderFacade: ProductModel
OrderFacade->>ProductService: decreaseStock(productId, quantity)
ProductService-->>OrderFacade: stock decreased
end
OrderFacade->>OrderService: createOrder(memberId, orderItems)
OrderService-->>OrderFacade: OrderModel
OrderFacade->>PointService: use(memberId, totalAmount)
PointService-->>OrderFacade: points deducted
OrderFacade-->>OrderController: OrderInfo
OrderController-->>Client: ApiResponse<OrderResponse>
sequenceDiagram
participant Client
participant LikeController
participant LikeFacade
participant MemberService
participant ProductService
participant ProductLikeService
Client->>LikeController: POST /api/v1/products/{id}/likes (headers: loginId, password)
LikeController->>LikeFacade: addLike(loginId, password, productId)
LikeFacade->>MemberService: getMyInfo(loginId, password)
MemberService-->>LikeFacade: MemberModel (authentication)
LikeFacade->>ProductService: getById(productId)
ProductService-->>LikeFacade: ProductModel (validation)
LikeFacade->>ProductLikeService: addLike(memberId, productId)
ProductLikeService-->>LikeFacade: like saved (or CONFLICT if duplicate)
LikeFacade-->>LikeController: void
LikeController-->>Client: ApiResponse<Void>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 리뷰 복잡도 근거: 신규 도메인 계층(모델, 저장소, 서비스, 파사드) 6개 엔티티에 걸친 광범위한 추가; 각 계층별 검증 로직 및 에러 처리 다양화; 다중 서비스 조율(주문-재고-포인트, 좋아요-회원-상품) 필요; 테스트 코드 규모 대(200+ 라인 테스트 클래스 다수). 다만 도메인별 구조가 일관성 있어 반복 패턴 인식 가능하다. Possibly related PRs
운영 관점 주요 지적1. 암호 검증 로직의 운영 위험성문제점: MemberModel 생성자에서 비밀번호 검증 시 생년월일 문자열 포함 여부를 확인하는데, 한글 처리 및 다국어 문자 처리가 미흡하다. 수정안:
추가 테스트: // MemberModelTest에 추가
`@Test`
void failWithBirthdateInPassword_비정규화된형식() {
LocalDate birthDate = LocalDate.of(1995, 3, 15);
// "1995-03-15", "95-03-15", "95031595" 등 다양한 형식 테스트
}2. 주문 생성 중 재고 감소와 포인트 차감 사이의 트랜잭션 일관성 부재문제점: OrderFacade.createOrder()에서 재고 감소 후 포인트 차감 전 실패 시, 재고는 감소했으나 주문은 생성되지 않는 부분 실패(partial failure) 발생 가능. Exception 발생 시 자동 롤백되지만, 각 단계의 세밀한 재시도 로직이 없다. 수정안:
추가 테스트: // OrderFacadeUnitTest에 추가
`@Test`
void createOrderRollbackOnPointDeductionFailure() {
// 1. 주문 생성 성공, 포인트 차감 실패
// 2. OrderService.createOrder() 미호출 검증
// 3. ProductService.decreaseStock() 롤백 검증
}3. 좋아요 CONFLICT 에러 코드의 명시성 부족문제점: ProductLikeService.addLike()에서 CONFLICT 예외 발생 시, 고객이 이미 좋아요를 누른 상태인지 다른 원인인지 구분 불가능. 에러 메시지가 한국어만 지원되어 국제화(i18n) 미흡. 수정안:
추가 테스트: // LikeFacadeUnitTest에 추가
`@Test`
void addLike_응답에중복감지메시지포함() {
// CONFLICT 발생 시 응답 구조 검증
// { "code": "LIKE_ALREADY_EXISTS", "productId": 123 } 형태 확인
}4. API 요청 헤더 인증의 보안 미흡문제점: X-Loopers-LoginId, X-Loopers-LoginPw를 평문 헤더로 전달하는 것은 HTTPS 필수 환경에서도 중간자 공격(MITM) 위험. 또한 요청별 비밀번호 검증 시 지연 공격(timing attack) 취약점 가능성 있다. 수정안:
추가 테스트: // MemberV1ApiE2ETest에 추가
`@Test`
void getMyInfo_비밀번호비교시간일정성() {
// 잘못된 비밀번호 응답 시간 측정
// 일관된 응답 시간 검증 (±10ms 범위)
}5. 재고 감소 로직의 동시성 문제 미해결문제점: ProductModel.decreaseStock()이 낙관적 동시성 제어(CAS)를 사용하지 않으므로, 동시 주문 시 초과 판매(overselling) 발생 가능. 현재 JPA의 일반 수정안:
추가 테스트: // ProductServiceUnitTest에 추가
`@Test`
void decreaseStock_동시다중요청() throws InterruptedException {
ExecutorService executor = Executors.newFixedThreadPool(10);
// 10개 스레드에서 동시 재고 감소 시뮬레이션
// OptimisticLockException 발생 및 처리 검증
}6. Point 사용 시 충분성 검증의 트랜잭션 격리 수준 미명시문제점: PointService.use()에서 포인트 조회 후 차감 전에 다른 트랜잭션이 포인트를 사용할 수 있는 TOCTOU(Time-of-check Time-of-use) 취약점 가능성. 수정안:
추가 테스트: // PointServiceUnitTest에 추가
`@Test`
`@Transactional`(isolation = Isolation.REPEATABLE_READ)
void use_동시다중포인트차감() {
// 두 트랜잭션에서 동시 use() 호출
// 두 번째 트랜잭션이 BAD_REQUEST 예외 발생 검증
}🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (7)
apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java-19-20 (1)
19-20:⚠️ Potential issue | 🟡 MinormemberId 중복 허용 가능성
memberId컬럼에unique제약이 없어, 동일 회원에 대해 복수의PointModel레코드가 생성될 수 있다. 비즈니스 로직상 회원당 하나의 포인트 레코드만 존재해야 한다면 DB 레벨에서 제약을 추가해야 한다.🛡️ 수정 제안
-@Column(nullable = false) +@Column(nullable = false, unique = true) private Long memberId;또는 별도 인덱스/제약을 스키마 마이그레이션에 추가한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java` around lines 19 - 20, PointModel currently allows duplicate memberId values; if business rules require one point record per member, add a uniqueness constraint for memberId—either annotate the entity (e.g., on class PointModel add `@Table`(uniqueConstraints = `@UniqueConstraint`(columnNames = "memberId")) or add `@Column`(unique = true) on the memberId field) or add a schema migration that creates a unique index on memberId; update any repository/service behavior to handle unique-constraint violations (e.g., translate DB exceptions) as needed.apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java-172-192 (1)
172-192:⚠️ Potential issue | 🟡 Minor비밀번호 변경 성공 테스트에 사후 인증 검증이 필요하다.
Line [172-192]는 200 응답만 확인해, 실제 비밀번호가 바뀌지 않았는데도 테스트가 통과할 수 있다. 운영에서 인증 장애가 배포 전 탐지되지 않을 위험이 있다.
수정안으로 변경 직후GET /api/v1/members/me를 새 비밀번호/기존 비밀번호로 각각 호출해 성공/실패를 함께 검증해야 한다.
추가 테스트로 새 비밀번호 인증은 200, 기존 비밀번호 인증은 400을 동시에 검증하는 단언을 넣어야 한다.🔧 제안 수정안
// then - assertThat(response.getStatusCode().is2xxSuccessful()).isTrue(); + assertThat(response.getStatusCode().is2xxSuccessful()).isTrue(); + + ParameterizedTypeReference<ApiResponse<MemberV1Dto.MyInfoResponse>> infoType = new ParameterizedTypeReference<>() {}; + ResponseEntity<ApiResponse<MemberV1Dto.MyInfoResponse>> newPwResponse = testRestTemplate.exchange( + ENDPOINT_ME, HttpMethod.GET, new HttpEntity<>(authHeaders(loginId, newPassword)), infoType + ); + ResponseEntity<ApiResponse<MemberV1Dto.MyInfoResponse>> oldPwResponse = testRestTemplate.exchange( + ENDPOINT_ME, HttpMethod.GET, new HttpEntity<>(authHeaders(loginId, currentPassword)), infoType + ); + assertAll( + () -> assertThat(newPwResponse.getStatusCode().is2xxSuccessful()).isTrue(), + () -> assertThat(oldPwResponse.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST) + );As per coding guidelines
**/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java` around lines 172 - 192, The changePasswordSuccess test currently only asserts a 2xx response from ENDPOINT_CHANGE_PASSWORD; update changePasswordSuccess to also verify post-change authentication: after memberFacade.register and the PATCH via testRestTemplate, call the "GET /api/v1/members/me" endpoint using authHeaders(loginId, newPassword) and assert a successful (200/2xx) response and expected member payload, then call the same endpoint with authHeaders(loginId, currentPassword) and assert a failure (400/4xx) response; use the existing testRestTemplate.exchange and appropriate ParameterizedTypeReference/ApiResponse DTOs and keep references to changePasswordSuccess, authHeaders, ENDPOINT_CHANGE_PASSWORD, memberFacade.register and testRestTemplate to locate the changes.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java-40-42 (1)
40-42:⚠️ Potential issue | 🟡 Minorname 길이 검증 누락
컬럼 정의에서
length = 50으로 제한되어 있으나, 생성자에서 길이 검증이 없다. 50자 초과 시 DB 레벨에서 예외가 발생하며, 사용자에게 명확한 오류 메시지를 제공하지 못한다.🛡️ 길이 검증 추가 제안
if (name == null || name.isBlank()) { throw new CoreException(ErrorType.BAD_REQUEST, "상품명은 비어있을 수 없습니다."); } +if (name.length() > 50) { + throw new CoreException(ErrorType.BAD_REQUEST, "상품명은 50자를 초과할 수 없습니다."); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java` around lines 40 - 42, ProductModel의 name 검증에서 null/blank 체크만 있고 DB 컬럼 정의(length = 50)를 초과하는 경우를 검증하지 않으므로, 생성자(또는 setName 메서드)에서 name.length()를 검사해 50자를 초과하면 CoreException(ErrorType.BAD_REQUEST, "상품명은 50자를 초과할 수 없습니다.")를 던지도록 추가해 주세요; 대상 식별자는 ProductModel 클래스의 name 필드와 해당 생성자/setName 메서드입니다.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java-68-73 (1)
68-73:⚠️ Potential issue | 🟡 MinorincreaseStock에서 정수 오버플로우 가능성
stockQuantity + quantity가Integer.MAX_VALUE를 초과하면 음수로 전환될 수 있다. 재고 복구 시나리오에서 대량 수량이 입력되면 데이터 정합성 문제가 발생한다.🛡️ 오버플로우 방어 제안
public void increaseStock(int quantity) { if (quantity <= 0) { throw new CoreException(ErrorType.BAD_REQUEST, "증가 수량은 1 이상이어야 합니다."); } + if ((long) this.stockQuantity + quantity > Integer.MAX_VALUE) { + throw new CoreException(ErrorType.BAD_REQUEST, "재고 수량이 최대치를 초과합니다."); + } this.stockQuantity += quantity; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java` around lines 68 - 73, The increaseStock method in ProductModel can overflow when this.stockQuantity + quantity exceeds Integer.MAX_VALUE; update increaseStock to guard against overflow by validating before adding: after the existing quantity > 0 check, verify that this.stockQuantity <= Integer.MAX_VALUE - quantity (or wrap the addition in Math.addExact and catch ArithmeticException) and if it would overflow throw a CoreException(ErrorType.BAD_REQUEST, "증가 수량이 너무 커서 재고를 증가시킬 수 없습니다.") so stockQuantity remains consistent; reference the increaseStock method and the stockQuantity field when making the change.apps/commerce-api/src/test/java/com/loopers/domain/product/ProductModelTest.java-150-196 (1)
150-196:⚠️ Potential issue | 🟡 Minor재고 증감의 음수 경계값 테스트가 누락되어 회귀를 놓칠 수 있다.
운영 중 도메인 검증이 약해져도(예:
<= 0에서== 0로 퇴행) 현재 테스트는 통과할 수 있어 잘못된 배포를 막지 못한다.decreaseStock(-1)과increaseStock(-1)의BAD_REQUEST를 명시적으로 추가하기 바란다.
추가 테스트로 음수 수량 입력 시CoreException(ErrorType.BAD_REQUEST)를 각각 검증하기 바란다.🔧 제안 수정안
@@ class DecreaseStock { + `@DisplayName`("차감 수량이 음수이면, BAD_REQUEST 예외가 발생한다.") + `@Test` + void failWithNegativeQuantity() { + ProductModel product = new ProductModel(1L, "에어맥스", "러닝화", 129000, 100, null); + + CoreException result = assertThrows(CoreException.class, () -> + product.decreaseStock(-1) + ); + + assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); + } } @@ class IncreaseStock { + `@DisplayName`("증가 수량이 음수이면, BAD_REQUEST 예외가 발생한다.") + `@Test` + void failWithNegativeQuantity() { + ProductModel product = new ProductModel(1L, "에어맥스", "러닝화", 129000, 100, null); + + CoreException result = assertThrows(CoreException.class, () -> + product.increaseStock(-1) + ); + + assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); + } }As per coding guidelines:
**/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/domain/product/ProductModelTest.java` around lines 150 - 196, Add explicit negative-boundary tests for ProductModel.decreaseStock and ProductModel.increaseStock: for each method add a test that calls decreaseStock(-1) and increaseStock(-1) respectively and assert that a CoreException is thrown with ErrorType.BAD_REQUEST; update the nested test classes (DecreaseStock and IncreaseStock) to include these negative-quantity failure cases so regression from "<= 0" to "== 0" will be caught.apps/commerce-api/src/main/java/com/loopers/application/member/MemberFacade.java-27-32 (1)
27-32:⚠️ Potential issue | 🟡 Minor회원 포인트 조회 시 포인트 정보 미존재 예외 처리가 필요하다.
pointService.getByMemberId(member.getId())가NOT_FOUND예외를 던질 수 있다. 이는 포인트 초기화가 실패했거나, 기존 회원 데이터에 포인트가 없는 경우 발생한다. 운영 중 이 상황이 발생하면 회원 정보 조회 자체가 실패하게 된다.수정안: 포인트가 없는 경우 기본값(0)을 반환하거나, 적절한 예외 메시지로 변환해야 한다.
추가 테스트: 포인트 정보가 없는 회원의 정보 조회 시나리오를 테스트에 추가해야 한다.
🛡️ 방어 로직 추가 제안
`@Transactional`(readOnly = true) public MemberInfo getMyInfo(String loginId, String password) { MemberModel member = memberService.getMyInfo(loginId, password); - PointModel point = pointService.getByMemberId(member.getId()); - return MemberInfo.of(member, point.getBalanceMoney()); + try { + PointModel point = pointService.getByMemberId(member.getId()); + return MemberInfo.of(member, point.getBalanceMoney()); + } catch (CoreException e) { + if (e.getErrorType() == ErrorType.NOT_FOUND) { + return MemberInfo.of(member, Money.of(0)); + } + throw e; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/member/MemberFacade.java` around lines 27 - 32, MemberFacade.getMyInfo currently calls pointService.getByMemberId(member.getId()) which can throw a NOT_FOUND when a member has no point record; update getMyInfo to handle that case by catching the specific exception (or testing for a null/absent result from pointService.getByMemberId) and substitute a default balance of 0 when missing before calling MemberInfo.of(member, balance). Ensure you reference pointService.getByMemberId and MemberInfo.of in the change and add a unit/integration test that verifies getMyInfo returns a MemberInfo with 0 balance for members without point records.apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItemModel.java-48-50 (1)
48-50:⚠️ Potential issue | 🟡 Minor
assignOrderId에 null 검증이 없다.
orderId가 null로 재할당되면 데이터 정합성 문제가 발생할 수 있다. 방어적으로 null 체크를 추가하는 것이 운영 안정성에 유리하다.🛡️ 방어적 검증 추가
public void assignOrderId(Long orderId) { + if (orderId == null) { + throw new CoreException(ErrorType.BAD_REQUEST, "주문 ID는 필수입니다."); + } this.orderId = orderId; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItemModel.java` around lines 48 - 50, The assignOrderId method in OrderItemModel should defensively reject null to avoid data consistency issues: in OrderItemModel.assignOrderId(Long orderId) add a null check (e.g., Objects.requireNonNull(orderId) or an explicit if (orderId == null) throw new IllegalArgumentException("orderId must not be null")) before assigning to this.orderId so null cannot be stored; keep the exception message clear to aid debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java`:
- Around line 61-65: Modify OrderFacade.getById to enforce ownership: accept the
requester identity (e.g., add a Long requesterId parameter or obtain it from the
security context inside getById), call orderService.getById(id) to load the
OrderModel, then compare order.getMemberId() with requesterId and throw an
authorization exception (e.g., AccessDeniedException or your app's equivalent)
if they differ before calling orderService.getOrderItems(...) and
OrderInfo.from(...); also add an API/facade test that attempts to read Member
A's order as Member B and asserts the authorization error is returned.
- Around line 31-39: Add explicit input validation at the start of createOrder:
if itemRequests is null or empty, or any OrderItemRequest has a null productId
or quantity <= 0, throw a CoreException with BAD_REQUEST; keep the rest of the
flow (memberService.getMyInfo, productService.getById,
productService.decreaseStock, building orderItems) unchanged and ensure all
error paths use CoreException so ApiControllerAdvice produces a consistent
response. Update unit tests for createOrder to cover null itemRequests, empty
list, an item with null productId, and items with quantity 0 or negative to
assert CoreException(BAD_REQUEST) is thrown.
In `@apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandModel.java`:
- Around line 28-35: The BrandModel constructor currently validates name
null/blank but misses length checks, causing DB integrity errors; update the
BrandModel(String name, String description, String imageUrl) constructor to
validate name.length() <= 50 and imageUrl.length() <= 500 and throw a
CoreException(ErrorType.BAD_REQUEST, "...") with clear messages when exceeded;
add unit tests covering boundary and out-of-bound cases (name lengths 50 and 51,
imageUrl lengths 500 and 501) and a normal-success case to ensure constructor
enforces the same limits declared earlier.
In `@apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.java`:
- Around line 20-23: Add input validation to BrandService.getById: before
calling brandRepository.findById(id) check if id == null || id <= 0 and throw
new CoreException(ErrorType.BAD_REQUEST, "유효하지 않은 브랜드 id입니다.") so that invalid
inputs are converted to domain errors instead of JPA exceptions; update unit
tests for getById to assert CoreException(ErrorType.BAD_REQUEST) and the message
when id is null, 0 and negative values. Ensure references to getById,
CoreException, ErrorType.BAD_REQUEST and brandRepository are used so the change
is localized to the service boundary.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/like/ProductLikeService.java`:
- Around line 15-23: The addLike method has a race condition between
productLikeRepository.findByMemberIdAndProductId and productLikeRepository.save
causing DataIntegrityViolationException on concurrent requests; update addLike
in ProductLikeService to catch DataIntegrityViolationException around the save
call and rethrow CoreException(ErrorType.CONFLICT, "이미 좋아요한 상품입니다.") to
normalize behavior (or alternatively document/use a pessimistic lock on the
member/product row); ensure DB unique constraint remains and add an integration
test that simulates concurrent calls to addLike (e.g.,
multi-thread/CountDownLatch) to verify the conversion.
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`:
- Around line 14-23: Add DB-level uniqueness and non-null constraints for
loginId and email on MemberModel: update the `@Table` annotation on class
MemberModel to declare uniqueConstraints for columns "login_id" and "email", and
mark the fields loginId and email with `@Column`(name="login_id", nullable=false)
and `@Column`(name="email", nullable=false) respectively (and ensure existing
mapping for password/name/birthDate remains unchanged); then add an integration
test that attempts concurrent inserts with the same loginId or email and asserts
a DataIntegrityViolationException is thrown to verify the DB enforces the
constraint.
- Line 3: MemberModel.java imports com.loopers.domain.BaseEntity but the actual
BaseEntity.java package declaration likely differs, causing build failure; open
BaseEntity.java to confirm its package and update the import in MemberModel
(referencing the import line in MemberModel.java and the package declaration in
BaseEntity.java) so they match, then recompile the :apps:commerce-api module and
run MemberModelTest to verify; you can use the provided verification commands to
print BaseEntity.java header and the top of MemberModel.java to confirm the fix.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`:
- Around line 20-27: The current pre-check using memberRepository.findByLoginId
in MemberService is race-prone; remove reliance on that as the sole guard and
instead ensure a DB UNIQUE constraint on loginId and wrap the save call
(memberRepository.save(new MemberModel(...)) / MemberModel constructor and
applyEncodedPassword) in a try-catch that converts DB constraint violations
(e.g., DataIntegrityViolationException, ConstraintViolationException,
DuplicateKeyException depending on JPA/DB) into a
CoreException(ErrorType.CONFLICT, "이미 존재하는 loginId입니다."); keep the semantic
check optional but rely on the DB for correctness, and add an integration test
that issues concurrent sign-up requests for the same loginId to assert one
success and the rest throw the CONFLICT CoreException.
In `@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderModel.java`:
- Line 18: Change the OrderModel.totalAmount field from int to long and update
any places that compute or mutate it (refer to OrderModel and the code around
the sum logic noted at lines 32-34) to use long arithmetic and
Math.addExact(...) to detect overflow, or refactor to use a Money/Amount value
object for accumulation; update constructors/getters/setters to preserve
immutability/encapsulation and add unit tests that exercise boundary/overflow
cases (large item prices, near-Integer.MAX_VALUE and Long limits) to assert
correct sums or that an overflow exception is thrown.
- Around line 28-34: In OrderModel's constructor/validation block, add an
explicit check that no element in orderItems is null (e.g., stream/loop over
orderItems and detect any null) and if any null found throw new
CoreException(ErrorType.BAD_REQUEST, "주문 항목은 1개 이상이어야 합니다." or a clearer message
like "주문 항목에 null이 포함되어 있습니다."); ensure totalAmount calculation
(OrderItemModel::getTotalAmount) only runs after this check so it cannot NPE,
and add a unit test that constructs orderItems containing a single null element
to assert that OrderModel creation throws CoreException with
ErrorType.BAD_REQUEST.
In `@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.java`:
- Around line 17-28: The createOrder method currently creates a new OrderModel
every call, causing duplicate orders on retries; change createOrder to accept an
idempotency key parameter, have it first query orderRepository for an existing
OrderModel by that key (e.g., add/findByIdempotencyKey), return the existing
order if found, otherwise create and persist a new OrderModel with the
idempotency key set, persist items with assignOrderId/saveItem as before, and
ensure the idempotency key is stored atomically with the order to prevent races
(use a unique constraint or transactional check-then-insert). Add a
unit/integration test that calls createOrder twice with the same idempotency key
and asserts only one order is created and both calls return the same order id.
In `@apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java`:
- Around line 39-54: The charge() and use() methods update the balance without
concurrency control, causing lost updates; add optimistic locking by introducing
a `@Version` field (e.g., private Long version) on the entity (or BaseEntity if
present) so JPA will detect concurrent modifications, and ensure repository
load/update flows honor version checks, or alternatively apply pessimistic
locking by annotating the repository read method
(PointRepository.findByMemberId(...)) with `@Lock`(LockModeType.PESSIMISTIC_WRITE)
so concurrent transactions serialize; update transaction boundaries where
charge() and use() are called to run in a transactional context so the
locking/versioning takes effect.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/like/ProductLikeJpaRepository.java`:
- Around line 9-10: Add a DB-level uniqueness constraint on (member_id,
product_id) and an index on product_id, update the JPA mapping/entity used by
ProductLikeJpaRepository to declare the unique constraint (so repository methods
like findByMemberIdAndProductId and countByProductId rely on DB guarantees),
catch the persistence/SQL unique-violation exception thrown on duplicate insert
in the service/repository layer and translate it into a consistent domain
exception (e.g., DuplicateProductLikeException) instead of letting a raw SQL
error leak, and add an integration test that concurrently attempts to save the
same (memberId, productId) and asserts only one insert succeeds and
countByProductId(productId) == 1 to validate the constraint.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/like/LikeV1ApiSpec.java`:
- Around line 11-23: The API methods addLike and removeLike expose credentials
via method parameters (loginId, loginPw); remove the loginPw parameter from
these methods and update the contract to expect authentication via an
Authorization token header instead (e.g., Bearer token), update the
`@Operation/`@Parameter annotations to reflect token usage (remove password param,
optionally mark loginId as derived from token or remove it if redundant), and
ensure controller/service layers that implement addLike/removeLike extract and
validate the token from the request header (or rely on standard
security/filter-based authentication) rather than performing per-call password
checks; also update any DTOs/tests/docs referencing loginPw.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java`:
- Around line 38-40: The controller currently accepts raw passwords via the
X-Loopers-LoginPw header (method parameters loginPw in MemberV1Controller),
which is insecure; change the endpoint to stop reading password from a
header—accept credentials in the request body (or better, switch to token-based
auth/JWT or session) and update the handler signature to a DTO (e.g.,
LoginRequest) instead of `@RequestHeader`("X-Loopers-LoginPw") String loginPw; if
you must keep backwards compatibility temporarily, ensure the code masks/omits
X-Loopers-LoginPw from any access/logging and add a deprecation notice to the
header usage so callers migrate to a secure auth method.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1ApiSpec.java`:
- Around line 18-19: Ensure getById enforces authentication and owner
authorization: update the API contract/interface for OrderV1ApiSpec.getById to
require an authenticated principal (e.g., add a security/Principal parameter or
annotate with required auth), then implement owner verification in the calling
layer (OrderFacade or OrderService) so that the authenticated user's ID is
compared to the order.ownerId before returning data; if IDs don't match return a
403. Touch the getById declaration and the OrderFacade/OrderService method that
fetches orders to perform this check and throw/return appropriate
unauthorized/forbidden responses.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.java`:
- Around line 31-33: Validate the incoming request and its items before mapping:
in OrderV1Controller, check that request != null and request.items() != null
(and optionally non-empty) and if the check fails throw new
CoreException(CoreException.ErrorCode.BAD_REQUEST) so the error is routed
through ApiControllerAdvice; update the mapping block that creates
List<OrderFacade.OrderItemRequest> to run after this validation and keep using
the existing OrderFacade.OrderItemRequest constructor, and add controller tests
that POST with items: null and with empty/absent body to assert the standard
error response is returned.
---
Minor comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/member/MemberFacade.java`:
- Around line 27-32: MemberFacade.getMyInfo currently calls
pointService.getByMemberId(member.getId()) which can throw a NOT_FOUND when a
member has no point record; update getMyInfo to handle that case by catching the
specific exception (or testing for a null/absent result from
pointService.getByMemberId) and substitute a default balance of 0 when missing
before calling MemberInfo.of(member, balance). Ensure you reference
pointService.getByMemberId and MemberInfo.of in the change and add a
unit/integration test that verifies getMyInfo returns a MemberInfo with 0
balance for members without point records.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItemModel.java`:
- Around line 48-50: The assignOrderId method in OrderItemModel should
defensively reject null to avoid data consistency issues: in
OrderItemModel.assignOrderId(Long orderId) add a null check (e.g.,
Objects.requireNonNull(orderId) or an explicit if (orderId == null) throw new
IllegalArgumentException("orderId must not be null")) before assigning to
this.orderId so null cannot be stored; keep the exception message clear to aid
debugging.
In `@apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java`:
- Around line 19-20: PointModel currently allows duplicate memberId values; if
business rules require one point record per member, add a uniqueness constraint
for memberId—either annotate the entity (e.g., on class PointModel add
`@Table`(uniqueConstraints = `@UniqueConstraint`(columnNames = "memberId")) or add
`@Column`(unique = true) on the memberId field) or add a schema migration that
creates a unique index on memberId; update any repository/service behavior to
handle unique-constraint violations (e.g., translate DB exceptions) as needed.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java`:
- Around line 40-42: ProductModel의 name 검증에서 null/blank 체크만 있고 DB 컬럼 정의(length =
50)를 초과하는 경우를 검증하지 않으므로, 생성자(또는 setName 메서드)에서 name.length()를 검사해 50자를 초과하면
CoreException(ErrorType.BAD_REQUEST, "상품명은 50자를 초과할 수 없습니다.")를 던지도록 추가해 주세요; 대상
식별자는 ProductModel 클래스의 name 필드와 해당 생성자/setName 메서드입니다.
- Around line 68-73: The increaseStock method in ProductModel can overflow when
this.stockQuantity + quantity exceeds Integer.MAX_VALUE; update increaseStock to
guard against overflow by validating before adding: after the existing quantity
> 0 check, verify that this.stockQuantity <= Integer.MAX_VALUE - quantity (or
wrap the addition in Math.addExact and catch ArithmeticException) and if it
would overflow throw a CoreException(ErrorType.BAD_REQUEST, "증가 수량이 너무 커서 재고를
증가시킬 수 없습니다.") so stockQuantity remains consistent; reference the increaseStock
method and the stockQuantity field when making the change.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/product/ProductModelTest.java`:
- Around line 150-196: Add explicit negative-boundary tests for
ProductModel.decreaseStock and ProductModel.increaseStock: for each method add a
test that calls decreaseStock(-1) and increaseStock(-1) respectively and assert
that a CoreException is thrown with ErrorType.BAD_REQUEST; update the nested
test classes (DecreaseStock and IncreaseStock) to include these
negative-quantity failure cases so regression from "<= 0" to "== 0" will be
caught.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java`:
- Around line 172-192: The changePasswordSuccess test currently only asserts a
2xx response from ENDPOINT_CHANGE_PASSWORD; update changePasswordSuccess to also
verify post-change authentication: after memberFacade.register and the PATCH via
testRestTemplate, call the "GET /api/v1/members/me" endpoint using
authHeaders(loginId, newPassword) and assert a successful (200/2xx) response and
expected member payload, then call the same endpoint with authHeaders(loginId,
currentPassword) and assert a failure (400/4xx) response; use the existing
testRestTemplate.exchange and appropriate ParameterizedTypeReference/ApiResponse
DTOs and keep references to changePasswordSuccess, authHeaders,
ENDPOINT_CHANGE_PASSWORD, memberFacade.register and testRestTemplate to locate
the changes.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java`:
- Around line 20-24: The addLike method currently calls
productService.getById(productId) which loads the full Product entity
unnecessarily; update addLike (in LikeFacade.addLike) to call a new
existence-checking method on ProductService (e.g., validateExists or existsById
wrapper) instead of getById, and implement that method in ProductService to use
productRepository.existsById(productId) and throw the existing
CoreException(ErrorType.NOT_FOUND, "상품이 존재하지 않습니다.") when false; ensure
LikeFacade still retrieves the member via memberService.getMyInfo and then calls
productLikeService.addLike(member.getId(), productId).
In
`@apps/commerce-api/src/main/java/com/loopers/application/member/MemberInfo.java`:
- Around line 15-22: Add explicit null guards in the static factory
MemberInfo.of(MemberModel model, Money point): validate that model and point are
not null and, following the module policy, throw a CoreException with a clear
message (e.g., "MemberModel must not be null" / "point must not be null") when
validation fails; ensure the exception type is CoreException so
ApiControllerAdvice will handle it uniformly. Update or add unit tests to assert
that MemberInfo.of(null, ...) and MemberInfo.of(validModel, null) throw
CoreException to cover the new behavior.
In
`@apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java`:
- Around line 73-77: The current OrderFacade mapping calls
orderService.getOrderItems(order.getId()) per order causing N+1 queries; add a
batch method like orderService.getOrderItemsByOrderIds(List<Long> orderIds) that
returns a List<OrderItemModel> (or Map<Long,List<OrderItemModel>>) and change
the stream to collect orderIds, call the batch method once, group returned items
by orderId in memory, then map orders to OrderInfo.from(order,
groupedItems.get(order.getId())); also add an integration test that creates ~100
orders and asserts the item-fetch query/call count does not grow linearly
(verifying batch behavior).
In
`@apps/commerce-api/src/main/java/com/loopers/application/order/OrderInfo.java`:
- Around line 34-41: Add defensive null checks to OrderInfo.from: validate that
the OrderModel parameter and the items list are non-null (e.g., use
Objects.requireNonNull(order, "order must not be null") and
Objects.requireNonNull(items, "items must not be null")) before constructing the
new OrderInfo; also guard against null elements in items by mapping with a
null-safe conversion (e.g., items.stream().map(i ->
Objects.requireNonNull(i)).map(OrderItemInfo::from).toList() or filter out
nulls) so OrderItemInfo.from is never called with null.
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductDetailInfo.java`:
- Around line 16-27: The factory method ProductDetailInfo.of currently assumes
product and brand are non-null and can NPE; add defensive null checks at the
start of ProductDetailInfo.of for ProductModel product and BrandModel brand and
throw a clear runtime exception (e.g., IllegalArgumentException or
NullPointerException) with messages like "product must not be null" / "brand
must not be null" so callers get a meaningful failure instead of an NPE; keep
the rest of the constructor call intact and reference ProductDetailInfo.of,
ProductModel, BrandModel, and likeCount when implementing the checks.
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java`:
- Around line 22-27: Clarify that the call to brandService.getById(brandId) in
ProductFacade.register is only for existence validation: either (A) replace the
unused call with a clear validator method like
brandService.validateExists(brandId) (add validateExists to BrandService and
implement it to throw when not found) or (B) keep getById but add a one-line
comment above the call stating "// validate brand exists" and assign/consume the
result to make the intent explicit; update ProductFacade.register accordingly to
call the new method or include the comment so the purpose is unambiguous.
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductInfo.java`:
- Around line 14-24: ProductInfo.from currently forwards ProductModel fields
without null checks and still uses an int price; update the method to validate
required fields from ProductModel (e.g., id, name, description/brandId if
non-nullable, imageUrl/stockQuantity according to domain rules) and either throw
a clear IllegalArgumentException or map to safe defaults when a required field
is null; also refactor the price handling to use the new Money value object
(convert model.getPrice() into Money via the Money factory/constructor) and
update ProductInfo's price field/constructor to accept Money instead of int so
currency/precision are preserved; ensure you reference ProductInfo.from,
ProductModel#getPrice(), and the Money VO when making these changes.
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductSortType.java`:
- Around line 10-12: Enum comparators (LATEST, PRICE_ASC, LIKES_DESC) are not
null-safe and LATEST uses id which may not reflect creation order; update these
to use null-safe comparators (e.g., Comparator.comparing(...,
Comparator.nullsLast(...)) or wrap key extractors with Objects::toString where
appropriate) for ProductDetailInfo::id, ProductDetailInfo::price and
ProductDetailInfo::likeCount, and change LATEST to sort by
ProductDetailInfo::createdAt (or a non-null timestamp getter) with a null-safe
comparator reversed for newest-first; add unit tests that cover null
id/price/likeCount/createdAt to validate behavior.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberRepository.java`:
- Around line 5-9: Add an ID-based lookup to the repository by adding a method
signature Optional<MemberModel> findById(Long id) to the MemberRepository
interface so callers (Order, Point, member update/delete flows) can retrieve
members by their numeric ID instead of using findByLoginId; then update any
concrete implementations (the class that implements MemberRepository) to provide
the corresponding lookup logic and ensure callers are switched to the new method
where appropriate to avoid unnecessary loginId-based queries.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderRepository.java`:
- Around line 6-12: Split the combined repository: extract OrderItem-related
methods (saveItem and findItemsByOrderId) into a new OrderItemRepository
interface and keep OrderRepository focused on OrderModel operations (save,
findById); update all usages to depend on the appropriate repository. Change
OrderRepository.findAllByMemberId(Long memberId) to accept pagination (e.g., add
Pageable parameter) or return a Page<OrderModel> to avoid loading full history;
update callers to pass pagination and adjust service/DAO logic accordingly. Add
integration tests that exercise fetching large volumes (≥1000 orders) via the
paginated method to validate memory and performance, and ensure compilation by
updating any wiring/DI to register the new OrderItemRepository.
In `@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.java`:
- Around line 22-25: The loop in OrderService that calls
orderRepository.saveItem(item) per OrderItemModel creates N DB round-trips and
becomes a bottleneck; modify OrderService to collect the orderItems, assign the
saved order id (saveOrder.getId()) to each item, and call a new batch
persistence method (e.g., orderRepository.saveItems(List<OrderItemModel>)) that
performs a single bulk insert/update within the same transaction; add or update
OrderRepository to implement saveItems efficiently (use JDBC batch, JPA bulk
persist, or repository-specific bulk API) and ensure transactional boundaries in
OrderService remain correct; finally add an integration test that creates a
100-item order and asserts no regression by checking query count or response
time versus baseline (or that only one batch insert call is executed).
In `@apps/commerce-api/src/main/java/com/loopers/domain/vo/Money.java`:
- Around line 16-22: The plus() and minus() methods in Money use
BigDecimal.add/subtract without enforcing scale or rounding, which can lead to
precision issues later; update Money.plus(Money) and Money.minus(Money) to apply
a consistent scale and rounding policy (e.g., define private static final int
SCALE and private static final RoundingMode ROUNDING) and call setScale(SCALE,
ROUNDING) on the result (or otherwise normalize operands before/after the
operation) so all Money results have the same scale and rounding behavior.
- Around line 7-14: The Money value object currently throws
IllegalArgumentException in its constructor checks, causing inconsistency with
other domain classes (e.g., BrandModel, PointModel) and the ApiControllerAdvice;
change the thrown exception to CoreException with ErrorType.BAD_REQUEST for both
null and negative amount checks so validation failures are handled by the
unified error pipeline (update the constructor in Money to throw new
CoreException(ErrorType.BAD_REQUEST) instead of IllegalArgumentException).
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemJpaRepository.java`:
- Line 9: OrderItemJpaRepository의 현재 findAllByOrderId 반복 호출로 인해 N+1이 발생하므로
OrderItemJpaRepository에 List<OrderItemModel> findAllByOrderIdIn(Collection<Long>
orderIds) 배치 조회 메서드를 추가하고, 주문 목록을 조립하는 상위 로직(예: 주문 조회/조립을 담당하는 서비스의
findAllByMemberId 또는 해당 어셈블러)에서 각 주문별로 개별 호출하지 않고 한 번에 orderIds를 전달해 모든 아이템을
조회하도록 교체하세요; 또한 통합 테스트를 추가해 N개 주문 조회 시 아이템 조회 쿼리 수가 상수 수준(즉, 배치 조회 하나)으로 유지되는지
검증하도록 하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java`:
- Around line 10-12: Replace the generic Spring stereotype with the
persistence-specific one: in the PointRepositoryImpl class annotate it with
`@Repository` instead of `@Component` so the infra layer is explicitly marked and
persistence exceptions are translated to Spring’s DataAccessException; update
imports accordingly (use org.springframework.stereotype.Repository and remove or
replace org.springframework.stereotype.Component) and keep the existing
`@RequiredArgsConstructor` and PointRepositoryImpl implementation unchanged.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/BrandV1Controller.java`:
- Around line 18-40: The BrandInfo→BrandV1Dto.BrandResponse mapping is
duplicated in register() and getById(); extract the mapping into a single
location (either add a static factory BrandV1Dto.BrandResponse.from(BrandInfo)
on BrandV1Dto.BrandResponse or a private helper method in BrandV1Controller) and
replace the inline constructions in register() and getById() with that single
call to ensure future response-field changes are made in one place.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Dto.java`:
- Around line 18-25: MyInfoResponse currently exposes the domain VO Money via
the point field; change the DTO to use an API-specific type (e.g.,
PointResponse) instead of Money to decouple layers: introduce a new record/class
PointResponse (with fields like amount and currency or whatever API contract you
need), map/convert Money -> PointResponse when constructing MyInfoResponse
(update any factory or mapper that builds MyInfoResponse), and remove direct
references to com.looopers...Money from the DTO so serialization and API schema
are controlled by the new API type.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Dto.java`:
- Around line 7-10: CreateRequest and other record types with List fields (e.g.,
CreateRequest(List<OrderItemRequest> items) and the records referenced at lines
18-24 and 36-39) must defensively copy incoming lists and enforce non-null to
guarantee immutability; add a compact constructor for each record that does
items = List.copyOf(Objects.requireNonNull(items, "items")); (or the equivalent
field name) so the DTO does not share mutable references, and add unit tests
asserting that mutating the original list after construction does not affect the
record and that attempts to modify the record's list throw
UnsupportedOperationException.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java`:
- Around line 64-72: Extract the sort-type validation into a static factory on
the enum: add ProductSortType.fromString(String) that wraps
ProductSortType.valueOf(...) and converts IllegalArgumentException into the
existing CoreException(ErrorType.BAD_REQUEST, ...) message listing valid values;
then simplify ProductV1Controller to call ProductSortType.fromString(sortType)
instead of catching IllegalArgumentException inline so validation is centralized
and reusable (refer to ProductSortType.valueOf, ProductSortType.fromString,
ProductV1Controller, CoreException, ErrorType.BAD_REQUEST).
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.java`:
- Around line 41-43: ProductListResponse should defensively copy and freeze the
incoming products list to prevent external mutation; add a compact constructor
in the record ProductListResponse that assigns this.products =
List.copyOf(products) (using java.util.List.copyOf) so the internal list is
immutable and detached from the original, and add unit tests that assert
modifying the source list after construction does not change ProductListResponse
and that the DTO's list throws on mutation attempts.
In
`@apps/commerce-api/src/test/java/com/loopers/application/member/MemberFacadeUnitTest.java`:
- Around line 131-151: Add boundary tests for chargePoint to assert that zero
and negative amounts propagate validation errors: create MemberModel with id set
(as in existing test), stub memberService.getMyInfo(...) to return it, and stub
pointService.charge(1L, Money.of(0)) and pointService.charge(1L, Money.of(-100))
to throw new CoreException(ErrorType.BAD_REQUEST, ...); then assert that calling
memberFacade.chargePoint("testuser", "password1!@", 0) and
memberFacade.chargePoint("testuser", "password1!@", -100) each throw
CoreException with ErrorType.BAD_REQUEST. Reference: memberFacade.chargePoint,
MemberModel, pointService.charge, Money.of, CoreException,
ErrorType.BAD_REQUEST.
In
`@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeUnitTest.java`:
- Around line 140-168: Add a verification that orderService.createOrder is never
called in the failWithInsufficientStock test: after asserting the CoreException
from orderFacade.createOrder, call Mockito.verify(orderService,
never()).createOrder(any(), any()) (or the project’s equivalent verify helper)
to explicitly assert no order is created when productService.decreaseStock
throws; update imports if necessary and reference the existing test method
failWithInsufficientStock in OrderFacadeUnitTest and the mocked
orderService/createOrder interaction.
- Around line 78-84: 주석 위치가 잘못되어 given-when-then 흐름이 깨져 있으므로
OrderFacadeUnitTest의 모킹 코드(예: memberService.getMyInfo(...),
productService.getById(...), brandService.getById(...),
orderService.createOrder(...), orderService.getOrderItems(...))는 그대로 두고 테스트의 실제
호출부 바로 앞(실제 메서드 호출을 수행하는 위치)으로 "// when" 주석을 옮기세요; 즉 given 블록에는 설정만 남기고 "//
when"은 실제 호출(테스트 대상 메서드 실행) 직전에 배치하여 given-when-then 패턴이 명확해지도록 수정하세요.
- Around line 58-169: Add unit tests in OrderFacadeUnitTest under the
CreateOrder nested class to cover boundary cases: (1) empty requests List.of() —
assert either an exception or that an empty OrderInfo is returned when calling
orderFacade.createOrder; (2) quantity <= 0 — call orderFacade.createOrder with
an OrderItemRequest having 0 and a negative quantity and assert CoreException
with ErrorType.BAD_REQUEST, ensuring productService.decreaseStock is not
invoked; (3) non-existent product ID — mock productService.getById to throw
CoreException(ErrorType.NOT_FOUND) and assert that orderFacade.createOrder
propagates that NOT_FOUND; and (4) multiple product items — provide multiple
OrderItemRequest entries, mock productService.getById and brandService/get
orderService as needed, then assert result.totalAmount equals the sum of each
(price * quantity) and verify productService.decreaseStock and pointService.use
are called with aggregated values; reference orderFacade.createOrder,
productService.getById, productService.decreaseStock, pointService.use, and
orderService.createOrder when adding these tests.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/brand/BrandModelTest.java`:
- Around line 15-56: Add missing boundary and negative tests for BrandModel in
BrandModelTest: include tests that construct BrandModel with description == null
and imageUrl == null and assert getters return null (use BrandModel constructor
and getters), add a max-length/name boundary test that creates a name at the DB
column limit and one exceeding it and assert expected behavior or exception
(refer to BrandModel constructor and CoreException/ErrorType), and add a test
that constructs a BrandModel with special characters/emojis in name and asserts
it is accepted or rejected per spec; place these new `@Test` methods alongside the
existing Create nested class so they run with the other creation tests.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/brand/BrandServiceUnitTest.java`:
- Around line 74-87: Expand the "getByIdSuccess" unit test in
BrandServiceUnitTest to assert the BrandModel's description and imageUrl in
addition to name, and add a verify(brandRepository).findById(id) to ensure the
repository was invoked; use the existing BrandModel instance and the
brandService.getById(id) call to perform these checks. Also add a separate unit
test that calls brandService.getById(null) (or the appropriate invalid id case)
and asserts the expected exception or error path to cover the null/invalid-id
boundary case. Ensure you reference BrandModel, brandRepository.findById, and
brandService.getById when making the updates.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/like/ProductLikeModelTest.java`:
- Around line 17-58: Add boundary-value unit tests around ProductLikeModel
construction: add tests that assert a CoreException with ErrorType.BAD_REQUEST
is thrown when memberId or productId are 0 or negative (e.g., new
ProductLikeModel(0L, 1L), new ProductLikeModel(1L, -1L)), and add a test that
verifies behavior for extreme values such as Long.MAX_VALUE (decide whether it
should succeed or throw and assert accordingly). Use the same pattern as
existing tests (assertThrows(CoreException.class, () -> new
ProductLikeModel(...)) and
assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST)) so the tests
reference the ProductLikeModel constructor and the ErrorType/ CoreException
behavior.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java`:
- Around line 250-308: Add two unit tests to cover null and blank new-password
boundary cases: create tests named failWithNullNewPassword and
failWithBlankNewPassword that instantiate MemberModel and call
member.changePassword(null) and member.changePassword(" "), respectively,
asserting a CoreException is thrown and its getErrorType() equals
ErrorType.BAD_REQUEST; place them alongside the existing password-failure tests
so changePassword null/blank validations are covered and prevent regressions.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java`:
- Around line 53-60: The test in MemberServiceIntegrationTest is brittle because
it compares result.getPassword() to PasswordEncoder.encode(password), which will
fail for salted/hard-to-predict encoders (e.g., BCrypt); instead, change the
assertion to verify password correctness via the encoder's verification API or
via the service's authentication flow: call PasswordEncoder.matches(password,
result.getPassword()) (or assert that getMyInfo/login succeeds using the stored
password) rather than comparing raw encoded strings; update references to
PasswordEncoder.encode(password) and the assertion on result.getPassword()
accordingly.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/order/OrderModelTest.java`:
- Around line 92-104: The test compares a Long-assigned orderId to an int
literal; update the assertion in OrderModelTest so the expected value matches
the Long type: change the assertion in assignOrderIdSuccess
(assertThat(item.getOrderId()).isEqualTo(...)) to use 100L to match the
assignOrderId(100L) call and avoid type mismatch.
- Around line 18-86: Add three boundary tests in OrderModelTest under the
CreateOrderItem nested class to cover missing/edge values: a test method (e.g.,
failWithNullBrandName) that constructs new OrderItemModel(1L, "에어맥스", null,
129000, 2) and asserts a CoreException with ErrorType.BAD_REQUEST; a test method
(e.g., failWithZeroPrice) that constructs new OrderItemModel(1L, "에어맥스", "나이키",
0, 2) and asserts a CoreException with ErrorType.BAD_REQUEST (or change the
assertion to success if domain allows zero and assert productPrice/totalAmount
accordingly); and a test method (e.g., failWithNegativeQuantity) that constructs
new OrderItemModel(1L, "에어맥스", "나이키", 129000, -1) and asserts a CoreException
with ErrorType.BAD_REQUEST. Ensure each test uses assertThrows/CoreException and
asserts the ErrorType like the existing tests.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceUnitTest.java`:
- Around line 31-61: Add negative unit tests to OrderServiceUnitTest to cover
null/empty input validation: in the CreateOrder nested class add tests that call
orderService.createOrder(null, items) and orderService.createOrder(1L,
Collections.emptyList()) (or null items) and assert that a CoreException is
thrown with ErrorType.BAD_REQUEST; ensure these tests reference the same
mocks/stubs as existing tests (orderRepository.save and saveItem) and verify no
repository saves occur for invalid inputs by asserting save/saveItem are not
invoked.
- Around line 98-118: Add unit tests for OrderService.getOrderItems(Long) in
OrderServiceUnitTest: create a nested test class (e.g., GetOrderItems) with a
success test that stubs orderRepository.findItemsByOrderId(1L) to return a
sample List<OrderItemModel> and asserts orderService.getOrderItems(1L) returns
that list, and add a failure/edge test that stubs findItemsByOrderId for a
non-existent ID (e.g., returns empty list or null per implementation) and
asserts the service handles it correctly (empty list or throws expected
exception); reference OrderService.getOrderItems and
orderRepository.findItemsByOrderId when adding these tests.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/point/PointModelTest.java`:
- Around line 65-78: Add two unit tests mirroring the existing
failWithZeroAmount pattern to assert that PointModel.charge(null) and
PointModel.use(null) throw CoreException with ErrorType.BAD_REQUEST: create
tests named failWithNullChargeAmount and failWithNullUseAmount, instantiate
PointModel (e.g., new PointModel(1L)), call assertThrows(CoreException.class, ()
-> point.charge(null)) / point.use(null), and
assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST) to lock the
null-input contract; follow the same assertion style as the existing
failWithZeroAmount test.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceUnitTest.java`:
- Around line 55-91: Add a unit test in PointServiceUnitTest to cover the
boundary case where a non-positive amount is charged: mock
pointRepository.findByMemberId(memberId) to return an Optional.of(new
PointModel(memberId)), then call pointService.charge(memberId, Money.zero()) and
assert that a CoreException is thrown with getErrorType() ==
ErrorType.BAD_REQUEST; ensure the new test method name and DisplayName reflect
the "0 이하 금액" case so the service-level propagation of domain validation
(PointModel) is verified.
- Around line 185-188: Remove the unnecessary trailing blank line at the end of
the PointServiceUnitTest file: open the class PointServiceUnitTest and delete
the extra empty line after the closing brace so the file ends immediately after
the final "}" to maintain file formatting consistency.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceUnitTest.java`:
- Around line 111-144: Add two unit tests in ProductServiceUnitTest's
DecreaseStock nested class to cover boundary/invalid inputs: assert that calling
productService.decreaseStock(1L, 0) and productService.decreaseStock(1L, -1)
each throw the expected CoreException (with ErrorType.BAD_REQUEST or whatever
error type decreaseStock uses for invalid quantities), mocking
productRepository.findById(1L) to return a valid ProductModel as in the other
tests; reference the decreaseStock method and the DecreaseStock test class to
locate where to add these cases.
In `@apps/commerce-api/src/test/java/com/loopers/domain/vo/MoneyTest.java`:
- Around line 117-126: Update the VO equality test in MoneyTest.equalByValue to
also assert the hashCode contract: after creating Money a = Money.of(1000) and
Money b = Money.of(1000), add an assertion that a.hashCode() == b.hashCode();
additionally add a new assertion (or new test) that for different values (e.g.,
Money.of(1000) vs Money.of(2000)) their hashCodes are not equal to reduce
regression risk; reference Money.of and the existing equalByValue test when
making these changes.
In `@http/commerce-api/member-v1.http`:
- Around line 6-7: Replace the real-looking password value in the HTTP test
payload (the "password" field in the member-v1 HTTP example) with a clearly
non-production test value such as "test-password-DO-NOT-USE"; update the example
JSON entry that currently has "password": "password1!@", ensuring only the
sample request body for loginId/password is changed and no functional code is
altered.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (95)
apps/commerce-api/src/main/java/com/loopers/application/brand/BrandFacade.javaapps/commerce-api/src/main/java/com/loopers/application/brand/BrandInfo.javaapps/commerce-api/src/main/java/com/loopers/application/example/ExampleFacade.javaapps/commerce-api/src/main/java/com/loopers/application/example/ExampleInfo.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.javaapps/commerce-api/src/main/java/com/loopers/application/member/MemberFacade.javaapps/commerce-api/src/main/java/com/loopers/application/member/MemberInfo.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderInfo.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductDetailInfo.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductInfo.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductSortType.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/BrandModel.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/BrandRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.javaapps/commerce-api/src/main/java/com/loopers/domain/example/ExampleModel.javaapps/commerce-api/src/main/java/com/loopers/domain/example/ExampleRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/example/ExampleService.javaapps/commerce-api/src/main/java/com/loopers/domain/like/ProductLikeModel.javaapps/commerce-api/src/main/java/com/loopers/domain/like/ProductLikeRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/like/ProductLikeService.javaapps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.javaapps/commerce-api/src/main/java/com/loopers/domain/member/MemberRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderItemModel.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderModel.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.javaapps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.javaapps/commerce-api/src/main/java/com/loopers/domain/point/PointRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/point/PointService.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.javaapps/commerce-api/src/main/java/com/loopers/domain/vo/Money.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/example/ExampleJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/example/ExampleRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/like/ProductLikeJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/like/ProductLikeRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/BrandV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/BrandV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/BrandV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/example/ExampleV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/example/ExampleV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/example/ExampleV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/like/LikeV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/like/LikeV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/like/LikeV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.javaapps/commerce-api/src/main/java/com/loopers/support/crypto/PasswordEncoder.javaapps/commerce-api/src/test/java/com/loopers/application/brand/BrandFacadeUnitTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeUnitTest.javaapps/commerce-api/src/test/java/com/loopers/application/member/MemberFacadeUnitTest.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeUnitTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeUnitTest.javaapps/commerce-api/src/test/java/com/loopers/domain/brand/BrandModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/brand/BrandServiceUnitTest.javaapps/commerce-api/src/test/java/com/loopers/domain/example/ExampleModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/example/ExampleServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/ProductLikeModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/ProductLikeServiceUnitTest.javaapps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceUnitTest.javaapps/commerce-api/src/test/java/com/loopers/domain/order/OrderModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceUnitTest.javaapps/commerce-api/src/test/java/com/loopers/domain/point/PointModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceUnitTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceUnitTest.javaapps/commerce-api/src/test/java/com/loopers/domain/vo/MoneyTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ExampleV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.javahttp/commerce-api/member-v1.http
💤 Files with no reviewable changes (13)
- apps/commerce-api/src/main/java/com/loopers/infrastructure/example/ExampleRepositoryImpl.java
- apps/commerce-api/src/test/java/com/loopers/interfaces/api/ExampleV1ApiE2ETest.java
- apps/commerce-api/src/main/java/com/loopers/domain/example/ExampleService.java
- apps/commerce-api/src/main/java/com/loopers/domain/example/ExampleModel.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/example/ExampleV1ApiSpec.java
- apps/commerce-api/src/main/java/com/loopers/application/example/ExampleFacade.java
- apps/commerce-api/src/test/java/com/loopers/domain/example/ExampleModelTest.java
- apps/commerce-api/src/main/java/com/loopers/infrastructure/example/ExampleJpaRepository.java
- apps/commerce-api/src/main/java/com/loopers/domain/example/ExampleRepository.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/example/ExampleV1Dto.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/example/ExampleV1Controller.java
- apps/commerce-api/src/main/java/com/loopers/application/example/ExampleInfo.java
- apps/commerce-api/src/test/java/com/loopers/domain/example/ExampleServiceIntegrationTest.java
Comment on lines
+31
to
+39
| public OrderInfo createOrder(String loginId, String password, List<OrderItemRequest> itemRequests) { | ||
| // 1. 회원 인증 | ||
| MemberModel member = memberService.getMyInfo(loginId, password); | ||
|
|
||
| // 2. 주문 항목 생성 (상품 스냅샷 + 재고 차감) | ||
| List<OrderItemModel> orderItems = new ArrayList<>(); | ||
| for (OrderItemRequest req : itemRequests) { | ||
| ProductModel product = productService.getById(req.productId()); | ||
| productService.decreaseStock(product.getId(), req.quantity()); |
There was a problem hiding this comment.
주문 항목 입력 검증 부재로 500 응답이 발생할 수 있다.
운영 환경에서 itemRequests가 null/empty이거나 항목의 productId가 null, quantity <= 0이면 현재는 NPE 또는 하위 예외로 흘러 일관된 오류 포맷이 깨질 수 있다. 메서드 시작 시 CoreException(BAD_REQUEST)로 명시 검증을 추가하기 바란다.
추가 테스트로 null, 빈 리스트, null productId, 0/음수 quantity 입력에 대한 createOrder 예외 케이스를 단위 테스트로 보강하기 바란다.
🔧 제안 수정안
`@Transactional`
public OrderInfo createOrder(String loginId, String password, List<OrderItemRequest> itemRequests) {
+ if (itemRequests == null || itemRequests.isEmpty()) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "주문 항목은 1개 이상이어야 한다.");
+ }
+ for (OrderItemRequest req : itemRequests) {
+ if (req == null || req.productId() == null || req.quantity() <= 0) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "주문 항목 정보가 올바르지 않다.");
+ }
+ }
// 1. 회원 인증
MemberModel member = memberService.getMyInfo(loginId, password);Based on learnings: In the loop-pack-be-l2-vol3-java project, enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format.
📝 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
| public OrderInfo createOrder(String loginId, String password, List<OrderItemRequest> itemRequests) { | |
| // 1. 회원 인증 | |
| MemberModel member = memberService.getMyInfo(loginId, password); | |
| // 2. 주문 항목 생성 (상품 스냅샷 + 재고 차감) | |
| List<OrderItemModel> orderItems = new ArrayList<>(); | |
| for (OrderItemRequest req : itemRequests) { | |
| ProductModel product = productService.getById(req.productId()); | |
| productService.decreaseStock(product.getId(), req.quantity()); | |
| public OrderInfo createOrder(String loginId, String password, List<OrderItemRequest> itemRequests) { | |
| if (itemRequests == null || itemRequests.isEmpty()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "주문 항목은 1개 이상이어야 한다."); | |
| } | |
| for (OrderItemRequest req : itemRequests) { | |
| if (req == null || req.productId() == null || req.quantity() <= 0) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "주문 항목 정보가 올바르지 않다."); | |
| } | |
| } | |
| // 1. 회원 인증 | |
| MemberModel member = memberService.getMyInfo(loginId, password); | |
| // 2. 주문 항목 생성 (상품 스냅샷 + 재고 차감) | |
| List<OrderItemModel> orderItems = new ArrayList<>(); | |
| for (OrderItemRequest req : itemRequests) { | |
| ProductModel product = productService.getById(req.productId()); | |
| productService.decreaseStock(product.getId(), req.quantity()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java`
around lines 31 - 39, Add explicit input validation at the start of createOrder:
if itemRequests is null or empty, or any OrderItemRequest has a null productId
or quantity <= 0, throw a CoreException with BAD_REQUEST; keep the rest of the
flow (memberService.getMyInfo, productService.getById,
productService.decreaseStock, building orderItems) unchanged and ensure all
error paths use CoreException so ApiControllerAdvice produces a consistent
response. Update unit tests for createOrder to cover null itemRequests, empty
list, an item with null productId, and items with quantity 0 or negative to
assert CoreException(BAD_REQUEST) is thrown.
Comment on lines
+61
to
+65
| @Transactional(readOnly = true) | ||
| public OrderInfo getById(Long id) { | ||
| OrderModel order = orderService.getById(id); | ||
| List<OrderItemModel> items = orderService.getOrderItems(order.getId()); | ||
| return OrderInfo.from(order, items); |
There was a problem hiding this comment.
주문 단건 조회에 소유권 검증이 없어 타회원 주문이 노출될 수 있다.
현재는 주문 ID만 알면 누구의 주문이든 조회 가능한 구조라 개인정보/주문정보 유출 위험이 있다. 조회 시 요청자 식별 정보를 받아 order.memberId == requesterId를 검증하고, 불일치 시 권한 오류를 반환하도록 수정하기 바란다.
추가 테스트로 회원 A의 주문을 회원 B가 조회할 때 권한 오류가 발생하는 케이스를 API/파사드 테스트에 포함하기 바란다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java`
around lines 61 - 65, Modify OrderFacade.getById to enforce ownership: accept
the requester identity (e.g., add a Long requesterId parameter or obtain it from
the security context inside getById), call orderService.getById(id) to load the
OrderModel, then compare order.getMemberId() with requesterId and throw an
authorization exception (e.g., AccessDeniedException or your app's equivalent)
if they differ before calling orderService.getOrderItems(...) and
OrderInfo.from(...); also add an API/facade test that attempts to read Member
A's order as Member B and asserts the authorization error is returned.
Comment on lines
+28
to
+35
| public BrandModel(String name, String description, String imageUrl) { | ||
| if (name == null || name.isBlank()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "브랜드명은 비어있을 수 없습니다."); | ||
| } | ||
| this.name = name; | ||
| this.description = description; | ||
| this.imageUrl = imageUrl; | ||
| } |
There was a problem hiding this comment.
도메인 생성자에서 컬럼 길이 경계 검증이 누락되어 운영 시 DB 예외로 실패할 수 있다.
Line [16]과 Line [22]에서 길이 제한을 정의했지만, Line [28]~Line [35] 생성자에서 이를 검증하지 않아 저장 시점에 DataIntegrityViolation로 터질 수 있다. 도메인 예외로 선제 차단해야 장애 분석과 응답 일관성이 좋아진다.
수정 시 name.length() <= 50, imageUrl.length() <= 500 검증을 추가하고, 테스트는 경계값(50/51, 500/501)과 정상값 케이스를 포함해야 한다.
🔧 제안 수정안
public BrandModel(String name, String description, String imageUrl) {
if (name == null || name.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "브랜드명은 비어있을 수 없습니다.");
}
+ if (name.length() > 50) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "브랜드명은 50자를 초과할 수 없습니다.");
+ }
+ if (imageUrl != null && imageUrl.length() > 500) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "브랜드 이미지 URL은 500자를 초과할 수 없습니다.");
+ }
this.name = name;
this.description = description;
this.imageUrl = imageUrl;
}As per coding guidelines **/domain/**/*.java: "엔티티/값 객체/DTO 경계를 명확히 하고, 불변성과 캡슐화를 점검한다."
📝 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
| public BrandModel(String name, String description, String imageUrl) { | |
| if (name == null || name.isBlank()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "브랜드명은 비어있을 수 없습니다."); | |
| } | |
| this.name = name; | |
| this.description = description; | |
| this.imageUrl = imageUrl; | |
| } | |
| public BrandModel(String name, String description, String imageUrl) { | |
| if (name == null || name.isBlank()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "브랜드명은 비어있을 수 없습니다."); | |
| } | |
| if (name.length() > 50) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "브랜드명은 50자를 초과할 수 없습니다."); | |
| } | |
| if (imageUrl != null && imageUrl.length() > 500) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "브랜드 이미지 URL은 500자를 초과할 수 없습니다."); | |
| } | |
| this.name = name; | |
| this.description = description; | |
| this.imageUrl = imageUrl; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandModel.java`
around lines 28 - 35, The BrandModel constructor currently validates name
null/blank but misses length checks, causing DB integrity errors; update the
BrandModel(String name, String description, String imageUrl) constructor to
validate name.length() <= 50 and imageUrl.length() <= 500 and throw a
CoreException(ErrorType.BAD_REQUEST, "...") with clear messages when exceeded;
add unit tests covering boundary and out-of-bound cases (name lengths 50 and 51,
imageUrl lengths 500 and 501) and a normal-success case to ensure constructor
enforces the same limits declared earlier.
Comment on lines
+20
to
+23
| @Transactional(readOnly = true) | ||
| public BrandModel getById(Long id) { | ||
| return brandRepository.findById(id) | ||
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 브랜드입니다.")); |
There was a problem hiding this comment.
getById 입력값 가드가 없어 예외 포맷이 비일관적으로 깨질 수 있다.
Line [22]에서 id가 null(또는 비정상 값)인 경우 JPA 계층 예외가 먼저 발생해 CoreException 기반 응답 포맷이 깨질 수 있다. 서비스 경계에서 입력값을 검증하고 도메인 예외로 변환해야 운영 장애 대응이 쉬워진다.
수정 시 id == null || id <= 0을 선검증하여 CoreException(ErrorType.BAD_REQUEST, ...)를 던지고, 테스트는 null/0/음수 id에 대한 예외 타입 및 메시지를 검증해야 한다.
🔧 제안 수정안
`@Transactional`(readOnly = true)
public BrandModel getById(Long id) {
+ if (id == null || id <= 0) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "유효하지 않은 브랜드 ID입니다.");
+ }
return brandRepository.findById(id)
.orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 브랜드입니다."));
}Based on learnings "enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format."
📝 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
| @Transactional(readOnly = true) | |
| public BrandModel getById(Long id) { | |
| return brandRepository.findById(id) | |
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 브랜드입니다.")); | |
| `@Transactional`(readOnly = true) | |
| public BrandModel getById(Long id) { | |
| if (id == null || id <= 0) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "유효하지 않은 브랜드 ID입니다."); | |
| } | |
| return brandRepository.findById(id) | |
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 브랜드입니다.")); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.java`
around lines 20 - 23, Add input validation to BrandService.getById: before
calling brandRepository.findById(id) check if id == null || id <= 0 and throw
new CoreException(ErrorType.BAD_REQUEST, "유효하지 않은 브랜드 id입니다.") so that invalid
inputs are converted to domain errors instead of JPA exceptions; update unit
tests for getById to assert CoreException(ErrorType.BAD_REQUEST) and the message
when id is null, 0 and negative values. Ensure references to getById,
CoreException, ErrorType.BAD_REQUEST and brandRepository are used so the change
is localized to the service boundary.
Comment on lines
+15
to
+23
| @Transactional | ||
| public void addLike(Long memberId, Long productId) { | ||
| productLikeRepository.findByMemberIdAndProductId(memberId, productId) | ||
| .ifPresent(like -> { | ||
| throw new CoreException(ErrorType.CONFLICT, "이미 좋아요한 상품입니다."); | ||
| }); | ||
|
|
||
| productLikeRepository.save(new ProductLikeModel(memberId, productId)); | ||
| } |
There was a problem hiding this comment.
addLike 동시성 이슈 (Race Condition)
findByMemberIdAndProductId 조회와 save 사이에 race condition이 존재한다. 두 요청이 동시에 같은 회원-상품 조합으로 좋아요를 시도하면:
- 두 요청 모두 빈 결과를 받음
- 두 요청 모두 save 시도
- DB unique constraint가 있으면 하나는
DataIntegrityViolationException발생 - 이 예외는
CoreException(CONFLICT)와 다르게 처리됨
수정안:
- DB 레벨 unique constraint +
DataIntegrityViolationException을CONFLICT로 변환하는 예외 처리 추가 - 또는 비관적 락 사용 (성능 트레이드오프 고려)
♻️ 예외 변환 예시
`@Transactional`
public void addLike(Long memberId, Long productId) {
productLikeRepository.findByMemberIdAndProductId(memberId, productId)
.ifPresent(like -> {
throw new CoreException(ErrorType.CONFLICT, "이미 좋아요한 상품입니다.");
});
try {
productLikeRepository.save(new ProductLikeModel(memberId, productId));
} catch (DataIntegrityViolationException e) {
throw new CoreException(ErrorType.CONFLICT, "이미 좋아요한 상품입니다.");
}
}추가 테스트: 동시 요청 시나리오를 재현하는 통합 테스트(멀티스레드 또는 CountDownLatch 활용) 권장
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/like/ProductLikeService.java`
around lines 15 - 23, The addLike method has a race condition between
productLikeRepository.findByMemberIdAndProductId and productLikeRepository.save
causing DataIntegrityViolationException on concurrent requests; update addLike
in ProductLikeService to catch DataIntegrityViolationException around the save
call and rethrow CoreException(ErrorType.CONFLICT, "이미 좋아요한 상품입니다.") to
normalize behavior (or alternatively document/use a pessimistic lock on the
member/product row); ensure DB unique constraint remains and add an integration
test that simulates concurrent calls to addLike (e.g.,
multi-thread/CountDownLatch) to verify the conversion.
Comment on lines
+9
to
+10
| Optional<ProductLikeModel> findByMemberIdAndProductId(Long memberId, Long productId); | ||
| long countByProductId(Long productId); |
There was a problem hiding this comment.
좋아요 중복을 DB 제약으로 고정해야 한다.
Line [9-10]은 조회 기반 중복 확인에는 유용하지만, 동시 요청에서는 동일 (memberId, productId)가 중복 저장되어 likeCount가 오염될 수 있다. 운영 환경에서 재시도 트래픽이 붙으면 데이터 정합성 이슈로 이어진다.
수정안으로 product_like(member_id, product_id) 유니크 제약과 product_id 인덱스를 추가하고, 유니크 충돌은 일관된 도메인 예외로 변환해야 한다.
추가 테스트로 동일 회원/상품 좋아요를 동시 저장하는 통합 테스트를 넣어 1건만 성공하고 countByProductId가 1인지 검증해야 한다.
As per coding guidelines **/*Repository*.java: 쿼리 조건 누락/과다 조회, 정렬/인덱스 활용 가능성, 대량 데이터에서의 병목을 점검한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/like/ProductLikeJpaRepository.java`
around lines 9 - 10, Add a DB-level uniqueness constraint on (member_id,
product_id) and an index on product_id, update the JPA mapping/entity used by
ProductLikeJpaRepository to declare the unique constraint (so repository methods
like findByMemberIdAndProductId and countByProductId rely on DB guarantees),
catch the persistence/SQL unique-violation exception thrown on duplicate insert
in the service/repository layer and translate it into a consistent domain
exception (e.g., DuplicateProductLikeException) instead of letting a raw SQL
error leak, and add an integration test that concurrently attempts to save the
same (memberId, productId) and asserts only one insert succeeds and
countByProductId(productId) == 1 to validate the constraint.
Comment on lines
+11
to
+23
| @Operation(summary = "좋아요 등록", description = "상품에 좋아요를 등록합니다.") | ||
| ApiResponse<Void> addLike( | ||
| @Parameter(description = "로그인 ID") String loginId, | ||
| @Parameter(description = "로그인 비밀번호") String loginPw, | ||
| @Parameter(description = "상품 ID") Long productId | ||
| ); | ||
|
|
||
| @Operation(summary = "좋아요 취소", description = "상품의 좋아요를 취소합니다.") | ||
| ApiResponse<Void> removeLike( | ||
| @Parameter(description = "로그인 ID") String loginId, | ||
| @Parameter(description = "로그인 비밀번호") String loginPw, | ||
| @Parameter(description = "상품 ID") Long productId | ||
| ); |
There was a problem hiding this comment.
인증 정보(loginPw) 전달 방식의 보안 위험
loginId와 loginPw를 API 메서드 파라미터로 직접 전달하는 방식은 다음과 같은 보안 문제를 야기할 수 있다:
- 로깅 노출: 요청 파라미터가 액세스 로그나 애플리케이션 로그에 평문으로 기록될 위험이 있다
- URL 노출: GET 요청이나 리다이렉트 시 URL에 비밀번호가 포함될 수 있다
- 감사 추적 어려움: 매 요청마다 인증을 수행하면 세션/토큰 기반 대비 성능 저하 및 감사 로그 관리가 복잡해진다
인증은 별도 인증 엔드포인트를 통해 토큰을 발급받고, 이후 요청에서는 Authorization 헤더로 토큰을 전달하는 방식이 표준적이다. 현재 구조가 의도된 설계라면, 최소한 파라미터명에서 loginPw 대신 인증 토큰 형태로 변경하는 것을 검토해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/like/LikeV1ApiSpec.java`
around lines 11 - 23, The API methods addLike and removeLike expose credentials
via method parameters (loginId, loginPw); remove the loginPw parameter from
these methods and update the contract to expect authentication via an
Authorization token header instead (e.g., Bearer token), update the
`@Operation/`@Parameter annotations to reflect token usage (remove password param,
optionally mark loginId as derived from token or remove it if redundant), and
ensure controller/service layers that implement addLike/removeLike extract and
validate the token from the request header (or rely on standard
security/filter-based authentication) rather than performing per-call password
checks; also update any DTOs/tests/docs referencing loginPw.
Comment on lines
+38
to
+40
| @RequestHeader("X-Loopers-LoginId") String loginId, | ||
| @RequestHeader("X-Loopers-LoginPw") String loginPw | ||
| ) { |
There was a problem hiding this comment.
HTTP 헤더를 통한 비밀번호 전송은 보안상 위험하다.
X-Loopers-LoginPw 헤더로 비밀번호를 전송하면 다음 문제가 발생한다:
- 웹 서버/프록시 액세스 로그에 평문 비밀번호 노출
- 브라우저 개발자 도구에서 헤더 노출
- 중간자 공격 시 헤더 탈취 용이
권장 수정안: 표준 인증 방식(JWT Bearer Token, Session-based auth)으로 전환을 고려해야 한다. 현재 구조를 유지해야 한다면 최소한 요청 본문(body)으로 이동하고, 액세스 로그에서 해당 헤더를 마스킹 처리해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java`
around lines 38 - 40, The controller currently accepts raw passwords via the
X-Loopers-LoginPw header (method parameters loginPw in MemberV1Controller),
which is insecure; change the endpoint to stop reading password from a
header—accept credentials in the request body (or better, switch to token-based
auth/JWT or session) and update the handler signature to a DTO (e.g.,
LoginRequest) instead of `@RequestHeader`("X-Loopers-LoginPw") String loginPw; if
you must keep backwards compatibility temporarily, ensure the code masks/omits
X-Loopers-LoginPw from any access/logging and add a deprecation notice to the
header usage so callers migrate to a secure auth method.
Comment on lines
+18
to
+19
| @Operation(summary = "주문 조회", description = "ID로 주문을 조회합니다.") | ||
| ApiResponse<OrderV1Dto.OrderResponse> getById(@Parameter(description = "주문 ID") Long id); |
There was a problem hiding this comment.
getById 인증 누락으로 인한 보안 취약점
getById는 인증 없이 주문 ID만으로 조회가 가능하다. 주문 정보에는 회원 ID, 주문 상품, 금액 등 민감 정보가 포함되어 있어, 인증된 사용자가 자신의 주문만 조회할 수 있도록 제한해야 한다.
🔒 인증 추가 제안
`@Operation`(summary = "주문 조회", description = "ID로 주문을 조회합니다.")
-ApiResponse<OrderV1Dto.OrderResponse> getById(`@Parameter`(description = "주문 ID") Long id);
+ApiResponse<OrderV1Dto.OrderResponse> getById(
+ `@Parameter`(description = "로그인 ID") String loginId,
+ `@Parameter`(description = "로그인 비밀번호") String loginPw,
+ `@Parameter`(description = "주문 ID") Long id
+);또는 주문 소유자 검증 로직을 Facade에 추가해야 한다.
📝 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
| @Operation(summary = "주문 조회", description = "ID로 주문을 조회합니다.") | |
| ApiResponse<OrderV1Dto.OrderResponse> getById(@Parameter(description = "주문 ID") Long id); | |
| `@Operation`(summary = "주문 조회", description = "ID로 주문을 조회합니다.") | |
| ApiResponse<OrderV1Dto.OrderResponse> getById( | |
| `@ParameterObject` `@RequestHeader`(name = "loginId", required = true) String loginId, | |
| `@ParameterObject` `@RequestHeader`(name = "loginPw", required = true) String loginPw, | |
| `@Parameter`(description = "주문 ID") Long id | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1ApiSpec.java`
around lines 18 - 19, Ensure getById enforces authentication and owner
authorization: update the API contract/interface for OrderV1ApiSpec.getById to
require an authenticated principal (e.g., add a security/Principal parameter or
annotate with required auth), then implement owner verification in the calling
layer (OrderFacade or OrderService) so that the authenticated user's ID is
compared to the order.ownerId before returning data; if IDs don't match return a
403. Touch the getById declaration and the OrderFacade/OrderService method that
fetches orders to perform this check and throw/return appropriate
unauthorized/forbidden responses.
Comment on lines
+31
to
+33
| List<OrderFacade.OrderItemRequest> itemRequests = request.items().stream() | ||
| .map(item -> new OrderFacade.OrderItemRequest(item.productId(), item.quantity())) | ||
| .toList(); |
There was a problem hiding this comment.
요청 본문의 items null 처리 누락으로 NPE가 발생할 수 있다.
운영에서 비정상 요청이 들어오면 파사드 진입 전에 NPE로 500이 발생해 표준 에러 응답이 깨질 수 있다. request와 request.items()를 먼저 검증하고, 위반 시 CoreException(BAD_REQUEST)를 던지도록 수정하기 바란다.
추가 테스트로 items: null 및 빈 본문 요청 시 표준 오류 응답이 반환되는 컨트롤러 테스트를 추가하기 바란다.
🔧 제안 수정안
+import com.loopers.support.error.CoreException;
+import com.loopers.support.error.ErrorType;
@@
public ApiResponse<OrderV1Dto.OrderResponse> createOrder(
`@RequestHeader`("X-Loopers-LoginId") String loginId,
`@RequestHeader`("X-Loopers-LoginPw") String loginPw,
`@RequestBody` OrderV1Dto.CreateRequest request
) {
+ if (request == null || request.items() == null || request.items().isEmpty()) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "주문 항목은 1개 이상이어야 한다.");
+ }
List<OrderFacade.OrderItemRequest> itemRequests = request.items().stream()
.map(item -> new OrderFacade.OrderItemRequest(item.productId(), item.quantity()))
.toList();Based on learnings: In the loop-pack-be-l2-vol3-java project, enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.java`
around lines 31 - 33, Validate the incoming request and its items before
mapping: in OrderV1Controller, check that request != null and request.items() !=
null (and optionally non-empty) and if the check fails throw new
CoreException(CoreException.ErrorCode.BAD_REQUEST) so the error is routed
through ApiControllerAdvice; update the mapping block that creates
List<OrderFacade.OrderItemRequest> to run after this validation and keep using
the existing OrderFacade.OrderItemRequest constructor, and add controller tests
that POST with items: null and with empty/absent body to assert the standard
error response is returned.
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.
한 줄 요약
Round 2의 설계 문서(요구사항 명세 · 시퀀스 · 클래스 다이어그램 · ERD)를 코드로 옮겼습니다.
6개 도메인, 15개 API, 22개 테스트 클래스(111개 테스트 메서드), 85개 파일 변경.
설계 -> 구현 정리
Round 2에서 작성한 설계 문서가 이번 구현에서 어디에 대응되는지 정리했습니다.
OrderFacade.createOrder()ProductModel.decreaseStock(),PointModel.use()등BaseEntity.deletedAt,OrderItemModel의 스냅샷 필드설계할 때는 "이렇게 하면 되겠지"였는데, 구현하면서 설계에 없던 것들이 필요해졌습니다.
MoneyVO가 대표적입니다. 금액 연산이PointModel곳곳에서 반복되는 것을 보고 뒤늦게 도입했습니다. 설계와 구현은 한 방향이 아니라 서로 피드백을 주고받는 관계라는 것을 느꼈습니다.구현 순서
커밋 로그를 보면 구현 순서가 드러납니다. 의도적으로 의존 방향의 안쪽(Domain)에서 바깥쪽(Interfaces)으로 진행했습니다.
이렇게 진행한 이유는, 안쪽 레이어가 확정되지 않은 상태에서 바깥을 먼저 만들면 안쪽이 바뀔 때마다 바깥도 같이 고쳐야 하기 때문입니다. Domain을 먼저 굳히고 그 위에 쌓아 올리니까, 수정이 한 방향으로만 전파되었습니다.
이번 과제에서 가장 고민한 3가지
Q1. "재고가 부족합니다"를 누가 말해야 하는가?
처음에는 서비스에서 검증하려 했습니다.
이 방식의 문제는 재고 관련 규칙이 서비스에 흩어진다는 것입니다. 주문에서도, 관리자 기능에서도 같은
if문이 반복됩니다. setter가 열리면 어디서든 재고를 바꿀 수 있게 됩니다.채택 이유: 규칙이 한 곳에만 존재합니다. 테스트할 때
new ProductModel(...)만으로 순수 단위 테스트가 가능합니다. 서비스는 "누구의 재고를 줄여라"만 전달하면 됩니다.이 결정이 프로젝트 전체의 톤을 정했습니다.
PointModel.use(),OrderModel생성자 등 모든 도메인 규칙을 같은 원칙으로 작성했습니다.Q2. Repository를 왜 세 겹으로 감싸야 하는가?
Spring Data JPA의
JpaRepository를 서비스에서 직접 쓰면 코드가 훨씬 적습니다. 그런데 이 프로젝트에서는 세 겹으로 나누었습니다.솔직히 처음에는 보일러플레이트가 아닌가 싶었습니다. 그런데 테스트를 작성하는 순간 이유를 체감했습니다.
JpaRepository에 직접 의존했다면 Mock 말고는 선택지가 없었을 것입니다. 인터페이스가 있으니까 Dummy, Stub, Mock, Spy, Fake — 5가지 테스트 더블을 전부 활용할 수 있었습니다. DIP(의존성 역전)가 교과서 개념이 아니라, 실제 테스트 코드에서 작동하는 것을 경험한 순간이었습니다.Q3. 주문할 때 상품 정보를 복사해야 하는가, 참조해야 하는가?
주문 항목에 상품 ID(FK)만 저장하면 간단합니다. 그런데 상품의 이름이나 가격이 나중에 바뀌면 어떻게 될까요? 3개월 전 주문 내역을 열었을 때 "현재 가격"이 뜨는 것은 잘못된 동작입니다.
결정:
productId는 추적용으로 남기되, 이름·브랜드명·가격은 주문 시점에 복사합니다. 이 설계는 Round 2 ERD에서 이미 결정했었는데, 실제OrderFacade에서 스냅샷 생성 코드를 작성할 때 "아, 이래서 ERD에서 이렇게 했구나" 하고 연결되는 경험이 있었습니다.API 목록 (15개)
인증이 필요한 API는
X-Loopers-LoginId+X-Loopers-LoginPw헤더를 사용합니다.POST/api/v1/membersGET/api/v1/members/mePATCH/api/v1/members/me/passwordPOST/api/v1/members/me/pointPOST/api/v1/brandsGET/api/v1/brands/{id}POST/api/v1/productsGET/api/v1/products/{id}GET/api/v1/products?sortType=LATEST&brandId=1POST/api/v1/products/{productId}/likesDELETE/api/v1/products/{productId}/likesGET/api/v1/likes/count?productId=1POST/api/v1/ordersGET/api/v1/orders/{id}GET/api/v1/orders/me테스트로 증명한 것들
단순히 "테스트를 작성했다"가 아니라, 각 테스트 레벨이 무엇을 증명하는지를 의식하며 작성했습니다.
Money.of(1000).plus(Money.of(2000))=Money.of(3000)특히
MemberServiceUnitTest에서는 테스트 더블 5종(Dummy, Stub, Mock, Spy, Fake)을 하나의 클래스에서 시연했습니다. 같은 서비스 로직을 다섯 가지 다른 관점으로 검증하면서, 각 테스트 더블의 용도와 차이를 코드로 보여주고자 했습니다.주요 데이터 흐름
주문 생성 시퀀스
sequenceDiagram participant C as Client participant Ctrl as OrderController participant F as OrderFacade participant MS as MemberService participant PS as ProductService participant PM as ProductModel participant BS as BrandService participant OS as OrderService participant PtS as PointService participant PtM as PointModel C->>Ctrl: POST /api/v1/orders Ctrl->>F: createOrder(loginId, pw, items) F->>MS: getMyInfo(loginId, pw) MS-->>F: MemberModel loop 각 주문 항목 F->>PS: getById(productId) PS-->>F: ProductModel F->>PS: decreaseStock(productId, qty) PS->>PM: decreaseStock(qty) Note over PM: 재고 부족 시 CoreException F->>BS: getById(brandId) BS-->>F: BrandModel Note over F: OrderItemModel 생성 (스냅샷) end F->>OS: createOrder(memberId, orderItems) OS-->>F: OrderModel (totalAmount 자동 계산) F->>PtS: use(memberId, totalAmount) PtS->>PtM: use(Money) Note over PtM: 포인트 부족 시 CoreException F-->>Ctrl: OrderInfo Ctrl-->>C: ApiResponse(SUCCESS)상품 목록 조회 — 세 도메인이 합쳐지는 지점
sequenceDiagram participant C as Client participant Ctrl as ProductController participant F as ProductFacade participant PS as ProductService participant BS as BrandService participant LS as ProductLikeService C->>Ctrl: GET /api/v1/products?sortType=LIKES_DESC Ctrl->>F: getProducts(LIKES_DESC) F->>PS: getAll() PS-->>F: List<ProductModel> loop 각 상품마다 F->>BS: getById(brandId) BS-->>F: BrandModel F->>LS: countByProductId(productId) LS-->>F: likeCount Note over F: ProductDetailInfo 조합 end Note over F: LIKES_DESC Comparator로 정렬 F-->>Ctrl: List<ProductDetailInfo> Ctrl-->>C: ApiResponse(ProductListResponse)그 외 설계 결정들
deletedAt)if-else분기가 없습니다@TransactionalMemberFacade에서 자동 생성CoreException+ErrorTypeenum구현하지 못한 것들
이번 주차 과제를 진행하면서 구현하지 못했던 부분들을 정리해보았습니다.
FOR UPDATE변경 파일 요약
총 85개 파일 변경 (+4,081 / -505 lines)