Conversation
WalkthroughUser/Store 조회 로직을 중앙화한 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant Service as 서비스 (Order/Product/Receipt/...)
participant Provider as UserContextProvider
participant Repo as UserRepository
participant DB as DB
rect rgb(235,248,255)
Note over Client,Service: 리팩토링 후의 주요 흐름
Client->>Service: API 호출 (userId, ...)
Service->>Provider: findUserAndStore(userId)
Provider->>Repo: findById(userId)
Repo->>DB: SELECT user
DB-->>Repo: User
Provider->>Provider: store = user.getStore()
Provider-->>Service: UserStoreContext(user, store)
Service->>Service: context.user()/context.store() 사용해 비즈니스 로직 실행
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30분 검토 시 특히 주의할 점:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/almang/inventory/global/context/UserContextProvider.java (1)
17-23: UserContextProvider로 사용자/매장 조회 공통화, 방향 좋습니다중복되던
userRepository.findById+user.getStore()로직을 한 곳으로 모은 구조가 깔끔합니다. 서비스들이 컨텍스트 객체만 의존하게 되어 이후 인증/권한 정책이 바뀌어도 이 컴포넌트만 수정하면 되는 점이 특히 좋습니다.다만 이 메서드가 이제 “단일 진입점”이 됐으니, 여기서 한 번에 유효성/예외를 정리해 두면 더 안전합니다.
- userId
null방어 코드 추가 제안현재
userId == null이면 JPA 구현에 따라findById(null)에서IllegalArgumentException등 예외가 날 수 있습니다. 도메인 예외로 감싸 두면 컨트롤러 단에서 일관된 에러 응답을 줄 수 있습니다.public UserStoreContext findUserAndStore(Long userId) { + if (userId == null) { + // 컨트롤러에서 이미 방어한다면 선택 사항이지만, + // 여기서 한 번 더 가드해 두면 예외 타입이 일관됩니다. + throw new BaseException(ErrorCode.USER_NOT_FOUND); + } + User user = userRepository.findById(userId) .orElseThrow(() -> new BaseException(ErrorCode.USER_NOT_FOUND)); Store store = user.getStore(); return new UserStoreContext(user, store); }
- store
null처리 여지도 이곳에서도메인 상 “항상 Store가 있는 사용자”만 온다는 전제가 아니라면,
user.getStore()가null인 경우도 이 메서드에서 도메인 예외(예:STORE_ACCESS_DENIED또는 별도 코드)로 통일해 두면, 각 서비스에서 NPE를 맞는 것보다 원인 파악이 쉬워질 것 같습니다. 필요해지면 같은 패턴으로 확장할 수 있을 것 같습니다.Also applies to: 25-25
src/main/java/com/almang/inventory/order/service/OrderService.java (1)
4-6: OrderService 전반에서 UserContextProvider 적용이 일관적입니다
- 각 퍼블릭 메서드가 모두
userContextProvider.findUserAndStore(userId)로 시작해서User/Store를 얻고, 이후에는 기존과 동일하게findVendorByIdAndValidateStore,findOrderByIdAndValidateAccess,validateOrderItemAccess등 “Store 기반 접근 제어”를 그대로 사용하고 있습니다. 기능/권한 체크 흐름이 그대로 유지된다는 점이 좋아 보입니다.- 로깅도 기존의
userId,storeId패턴을 유지하면서 context에서 가져온Store를 활용하고 있어, 운영 중 로그 분석에도 변화가 거의 없을 것 같습니다.개선 여지로 볼 수 있는 점은 다음 정도입니다 (필수는 아닙니다):
- 여러 메서드에서 아래와 같은 패턴이 반복됩니다.
만약 이후에도 이 패턴이 더 늘어난다면, 내부 헬퍼 메서드 예를 들어UserStoreContext context = userContextProvider.findUserAndStore(userId); Store store = context.store();정도로 감싸 두면 서비스 코드가 조금 더 읽기 쉬워질 수 있습니다. 지금 상태도 충분히 명료해서 “선택적 리팩터링” 수준으로 보입니다.private Store getStoreByUserId(Long userId) { return userContextProvider.findUserAndStore(userId).store(); }이런 형태의 컨텍스트 Provider를 통해 서비스 레이어에서 Repository 의존을 얇게 만드는 패턴은, Spring Data JPA 레퍼런스의 서비스 계층 분리나 DDD 애플리케이션 서비스 설계와도 잘 맞는 방향입니다.
Also applies to: 52-52, 55-59, 78-81, 94-96, 108-110, 129-131, 142-144, 165-167
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/almang/inventory/global/context/UserContextProvider.java(1 hunks)src/main/java/com/almang/inventory/inventory/service/InventoryService.java(0 hunks)src/main/java/com/almang/inventory/order/service/OrderService.java(8 hunks)src/main/java/com/almang/inventory/product/service/ProductService.java(5 hunks)src/main/java/com/almang/inventory/receipt/service/ReceiptService.java(11 hunks)src/main/java/com/almang/inventory/store/service/StoreService.java(3 hunks)src/main/java/com/almang/inventory/vendor/service/VendorService.java(7 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/almang/inventory/inventory/service/InventoryService.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-20T10:43:47.502Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 33
File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66
Timestamp: 2025-11-20T10:43:47.502Z
Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
Applied to files:
src/main/java/com/almang/inventory/product/service/ProductService.javasrc/main/java/com/almang/inventory/vendor/service/VendorService.javasrc/main/java/com/almang/inventory/store/service/StoreService.java
📚 Learning: 2025-11-22T18:12:13.172Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 66
File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53
Timestamp: 2025-11-22T18:12:13.172Z
Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates.
<!--
Applied to files:
src/main/java/com/almang/inventory/product/service/ProductService.javasrc/main/java/com/almang/inventory/order/service/OrderService.javasrc/main/java/com/almang/inventory/store/service/StoreService.java
🔇 Additional comments (4)
src/main/java/com/almang/inventory/product/service/ProductService.java (1)
4-5: ProductService도 컨텍스트 기반으로 잘 정리되었습니다
createProduct,updateProduct,getProductDetail,getProductList모두 공통적으로UserStoreContext를 통해 유저/점포 정보를 얻고, 이후
findVendorByIdAndValidateAccess(vendorId, user)validateStoreAccess(product, user)findProductsByFilter(store.getId(), …)
등 기존의 접근 제어 로직을 그대로 사용하고 있습니다. 결과적으로 권한·점포 검증 방식은 유지하면서, 조회 책임만 UserContextProvider로 위임된 구조입니다.- 특히
toEntity/findVendorByIdAndValidateAccess가 여전히user.getStore()를 기준으로 검증하고 있어서, 이전 PR들에서 정리해 둔 “서비스에서 먼저 유효성 검사 후 엔티티 업데이트” 패턴과도 잘 어울립니다.전반적으로 가독성도 좋고, 중복 제거도 자연스럽게 해결된 리팩터링으로 보입니다. 중복 잡는 한 방이네요.
Also applies to: 35-35, 39-41, 53-55, 74-76, 87-88
src/main/java/com/almang/inventory/receipt/service/ReceiptService.java (1)
4-5: ReceiptService에서 store 접근 로직이 깨끗하게 통일되었습니다
createReceiptFromOrder,getReceiptFromOrder,getReceipt,getReceiptList,updateReceipt,deleteReceipt,confirmReceipt,getReceiptItem,updateReceiptItem,deleteReceiptItem모두 시작 부분에서패턴으로UserStoreContext context = userContextProvider.findUserAndStore(userId); Store store = context.store();Store를 구한 뒤, 나머지는 기존처럼
findOrderByIdAndValidateAccess(orderId, store)findReceiptByIdAndValidateAccess(receiptId, store)findReceiptsByFilter(store.getId(), …)findReceiptItemByIdAndValidateAccess(receiptItemId, receipt)
를 통해 접근 제어를 수행하고 있습니다.- 결과적으로 입고/발주/입고 아이템에 대한 “해당 매장의 데이터만 볼 수 있다”는 규칙이 그대로 유지되고, 사용자·매장 조회만 중앙 집중화된 형태라 안전한 리팩터링으로 판단됩니다.
- 접근 제어 전용 private 메서드들(
findOrderByIdAndValidateAccess,findReceiptByIdAndValidateAccess,findReceiptItemByIdAndValidateAccess) 덕분에 이번 변경의 영향 범위도 상당히 제한적이라, 향후 정책 변경 시에도 수정 포인트가 명확할 것 같습니다.깔끔하게 패턴을 맞추신 덕분에 Receipt 쪽도 읽기 수월합니다.
Also applies to: 51-51, 55-57, 76-78, 91-93, 106-107, 121-123, 142-144, 160-162, 178-180, 198-200, 220-222
src/main/java/com/almang/inventory/store/service/StoreService.java (1)
4-5: StoreService의 사용자→상점 매핑도 UserContextProvider로 잘 일원화되었습니다
updateStore/getStoreOrderTemplateList에서 모두로 상점을 구한 뒤,UserStoreContext context = userContextProvider.findUserAndStore(userId); Store store = context.store();
- 상점 정보 변경은
validateStoreName,validateDefaultCountCheckThreshold후store.update…- 발주 템플릿 조회는
findStoreOrderTemplatesByFilter(store.getId(), …)
로 이어지는 기존 흐름을 그대로 유지하고 있습니다.- 즉, “어떤 Store에 대한 요청인가?”를 결정하는 책임만 UserContextProvider로 이동했고, Store 엔티티에 대한 검증/비즈니스 로직은 그대로 StoreService에 남아 있어 책임 분리가 자연스럽습니다.
앞단 인증 방식이 바뀌어도 StoreService는
userId → Store매핑 전략을 신경 쓰지 않아도 되는 구조라, 장기적으로 유지보수에도 유리해 보입니다.Also applies to: 30-30, 34-36, 61-63
src/main/java/com/almang/inventory/vendor/service/VendorService.java (1)
4-5: VendorService도 컨텍스트 기반 접근으로 자연스럽게 교체되었습니다
createVendor,updateVendor,getVendorDetail,getVendorList,createOrderTemplate,getOrderTemplates모두에서로 시작해,UserStoreContext context = userContextProvider.findUserAndStore(userId); User user = context.user(); // 또는 Store store = context.store();
findVendorByIdAndValidateAccess(vendorId, user)findVendorsByFilter(store.getId(), …)findOrderTemplatesByFilter(vendor.getId(), activated)
를 호출하는 구조로 정리되어 있습니다.- 특히
findVendorByIdAndValidateAccess가 여전히vendor.getStore().getId().equals(user.getStore().getId())를 기준으로 접근 권한을 판별하므로, “해당 유저의 상점에 속한 발주처만 접근 가능”이라는 기존 규칙이 그대로 유지됩니다.- 컨텍스트 도입으로 VendorService는 “유저가 어떤 Store에 속해 있는가?”를 알 필요 없이
User/Store를 이미 검증된 형태로 받게 되어, 서비스 레벨 책임이 한결 명확해진 점도 좋습니다.전반적으로 다른 서비스들과 패턴이 잘 맞춰져 있어, 이후 기능 추가 시에도 일관된 코드를 유지하기 쉬워 보입니다.
Also applies to: 35-35, 39-41, 52-54, 67-69, 79-81, 93-95, 107-108
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java (1)
1486-1512: 테스트 패턴 일관성을 위해 기존 헬퍼 메서드 사용을 고려하세요.현재 Product, Order, OrderItem을 인라인으로 생성하고 있지만, 파일 내 대부분의 테스트는
newOrderWithItems()헬퍼 메서드를 사용합니다. 인라인 생성 방식이 특별히 필요한 경우가 아니라면 기존 패턴을 따르는 것이 유지보수에 유리합니다.기존 헬퍼 메서드를 사용하는 방식으로 단순화할 수 있습니다:
// given Store store = newStore("재고확인상점"); User user = newUser(store, "inventoryUser"); Vendor vendor = newVendor(store, "재고발주처"); - Product product = newProduct(store, vendor, "상품1", "P001"); - - Inventory inventory = newInventory(product); - inventory.increaseIncoming(BigDecimal.valueOf(5)); - - Order order = Order.builder() - .store(store) - .vendor(vendor) - .status(OrderStatus.REQUEST) - .orderMessage("테스트 발주") - .activated(true) - .totalPrice(0) - .build(); - - OrderItem item = OrderItem.builder() - .product(product) - .quantity(5) - .unitPrice(1000) - .amount(5000) - .build(); - - order.addItem(item); - orderRepository.save(order); + Order order = newOrderWithItems(store, vendor); + + for (OrderItem orderItem : order.getItems()) { + Inventory inventory = newInventory(orderItem.getProduct()); + inventory.increaseIncoming(BigDecimal.valueOf(orderItem.getQuantity())); + inventoryRepository.save(inventory); + }단, 특정 시나리오(단일 아이템만 필요한 경우 등)를 테스트해야 한다면 현재 방식도 타당합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-22T18:12:13.172Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 66
File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53
Timestamp: 2025-11-22T18:12:13.172Z
Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates.
<!--
Applied to files:
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java
Show resolved
Hide resolved
42a10fa to
0336471
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java (1)
1480-1525: 입고 확정 성공 테스트의 픽스처 구성이 도메인 의도를 잘 드러납니다단일 상품/재고/발주/입고로 흐름을 구성해서
confirmReceipt의 성공 시나리오만 깔끔하게 검증하고 있고, 재고 반영 로직은 아래 두 테스트에서 별도로 커버하고 있어서 역할 분리가 명확합니다. 짧지만 이런 식의 “한 가지 책임만 검증하는” 테스트 구조는 유지보수에 유리합니다.선택사항이지만, 다음 정도만 고민해 보면 좋겠습니다.
- 이 테스트는 상태 전이만 검증하고, 바로 아래 테스트들이 재고 수량까지 검증하므로, 의도를 주석으로 한 줄만 남겨 두면 (예: “재고 수량 검증은 아래 테스트에서 수행”) 이후 도메인 규칙이 복잡해져도 테스트 독자가 헷갈리지 않습니다.
- 현재
newInventory(product)+increaseIncoming방식은 JPA 더티 체킹에 의존하는 패턴이라, 팀 내에서 이 관례를 공유해 두거나 Spring Data JPA 레퍼런스 문서를 한 번쯤 같이 보는 것도 좋겠습니다.전반적으로 이 변경으로 시나리오가 훨씬 읽기 쉬워졌습니다. 👍
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/almang/inventory/global/context/UserContextProvider.java(1 hunks)src/main/java/com/almang/inventory/inventory/service/InventoryService.java(0 hunks)src/main/java/com/almang/inventory/order/service/OrderService.java(8 hunks)src/main/java/com/almang/inventory/product/service/ProductService.java(5 hunks)src/main/java/com/almang/inventory/receipt/service/ReceiptService.java(11 hunks)src/main/java/com/almang/inventory/store/service/StoreService.java(3 hunks)src/main/java/com/almang/inventory/vendor/service/VendorService.java(7 hunks)src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/almang/inventory/inventory/service/InventoryService.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-22T18:12:13.172Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 66
File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53
Timestamp: 2025-11-22T18:12:13.172Z
Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates.
<!--
Applied to files:
src/main/java/com/almang/inventory/order/service/OrderService.javasrc/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.javasrc/main/java/com/almang/inventory/product/service/ProductService.javasrc/main/java/com/almang/inventory/store/service/StoreService.java
📚 Learning: 2025-11-20T10:43:47.502Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 33
File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66
Timestamp: 2025-11-20T10:43:47.502Z
Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
Applied to files:
src/main/java/com/almang/inventory/product/service/ProductService.javasrc/main/java/com/almang/inventory/store/service/StoreService.javasrc/main/java/com/almang/inventory/vendor/service/VendorService.java
🔇 Additional comments (7)
src/main/java/com/almang/inventory/global/context/UserContextProvider.java (2)
11-26: 깔끔한 리팩토링입니다! 👍User/Store 조회 로직을 중앙화하여 코드 중복을 효과적으로 제거했습니다.
record를 활용한UserStoreContext는 불변성과 간결성을 모두 갖춘 좋은 선택입니다.Spring의
@Component와 Lombok의@RequiredArgsConstructor조합도 적절합니다.
17-23: Store field is guaranteed to be non-null by JPA constraintThe User entity has
@JoinColumn(name = "store_id", nullable = false)on the Store field (line 39 of User.java). This database-level constraint ensures the foreign key column cannot be NULL, souser.getStore()will never return null for valid User records fetched from the database. No additional null check is required in thefindUserAndStore()method.Likely an incorrect or invalid review comment.
src/main/java/com/almang/inventory/order/service/OrderService.java (1)
52-58: 일관된 리팩토링 적용 확인 완료 ✓
UserContextProvider를 통한 컨텍스트 기반 접근 방식이 모든 메서드(createOrder,getOrder,getOrderList,updateOrder등)에 일관되게 적용되었습니다.기존 비즈니스 로직과 유효성 검증 흐름이 그대로 유지되어 리팩토링으로 인한 기능 변경 없이 코드 중복만 제거되었습니다.
src/main/java/com/almang/inventory/store/service/StoreService.java (1)
30-35: 간결하게 정리되었습니다!
UserContextProvider적용으로updateStore와getStoreOrderTemplateList메서드가 깔끔해졌습니다. 기존 유효성 검증 로직(validateStoreName,validateDefaultCountCheckThreshold)은 그대로 유지되어 안정성을 보장합니다.src/main/java/com/almang/inventory/receipt/service/ReceiptService.java (1)
51-56: 모든 입고 관련 메서드에 일관된 패턴 적용 확인 ✓
createReceiptFromOrder부터deleteReceiptItem까지 10개의 public 메서드에UserContextProvider패턴이 일관되게 적용되었습니다.재고 서비스(
InventoryService)와의 연동 로직과 입고 확정 시 재고 상태 변경 로직이 그대로 유지되어 기능적 안정성이 보장됩니다.src/main/java/com/almang/inventory/vendor/service/VendorService.java (1)
35-44: 발주처 서비스 리팩토링 완료 ✓
createVendor,updateVendor,getVendorDetail,getVendorList,createOrderTemplate,getOrderTemplates모든 메서드에 컨텍스트 기반 접근 방식이 적용되었습니다.
findVendorByIdAndValidateAccess메서드를 통한 접근 권한 검증 로직도 잘 유지되고 있습니다.src/main/java/com/almang/inventory/product/service/ProductService.java (1)
35-49: 품목 서비스 리팩토링 완료 ✓
createProduct,updateProduct,getProductDetail,getProductList메서드에UserContextProvider패턴이 일관되게 적용되었습니다.품목 생성 시
inventoryService.createInventory(saved)호출과findVendorByIdAndValidateAccess를 통한 벤더 접근 검증 로직이 그대로 유지되어 안전합니다.
✨ 작업 내용
📝 적용 범위
/global/context/product/order/store/vendor/receipt📌 참고 사항
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.