Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public enum SuccessMessage {
CREATE_VENDOR_SUCCESS("발주처 등록 성공"),
UPDATE_VENDOR_SUCCESS("발주처 수정 성공"),
GET_VENDOR_DETAIL_SUCCESS("발주처 상세 조회 성공"),
GET_VENDOR_LIST_SUCCESS("발주처 목록 조회 성공"),
;

private final String message;
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/com/almang/inventory/global/util/PaginationUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.almang.inventory.global.util;

import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Sort;
import org.springframework.data.domain.Sort.Direction;

public class PaginationUtil {

private PaginationUtil() {} // 인스턴스 생성 방지

public static PageRequest createPageRequest(Integer page, Integer size, String sortBy) {
int pageIndex = (page == null || page < 1) ? 0 : page - 1;
int pageSize = (size == null || size < 1) ? 20 : size;

return PageRequest.of(pageIndex, pageSize, Sort.by(Direction.ASC, sortBy));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.almang.inventory.global.api.PageResponse;
import com.almang.inventory.global.exception.BaseException;
import com.almang.inventory.global.exception.ErrorCode;
import com.almang.inventory.global.util.PaginationUtil;
import com.almang.inventory.product.domain.Product;
import com.almang.inventory.product.dto.request.CreateProductRequest;
import com.almang.inventory.product.dto.request.UpdateProductRequest;
Expand Down Expand Up @@ -80,7 +81,7 @@ public PageResponse<ProductResponse> getProductList(
Store store = user.getStore();

log.info("[ProductService] 품목 목록 조회 요청 - userId: {}, storeId: {}", userId, store.getId());
PageRequest pageable = createPageRequest(page, size);
PageRequest pageable = PaginationUtil.createPageRequest(page, size, "name");
Page<Product> productPage = findProductsByFilter(store.getId(), isActivate, nameKeyword, pageable);
Page<ProductResponse> mapped = productPage.map(ProductResponse::from);

Expand Down Expand Up @@ -134,13 +135,6 @@ private void validateStoreAccess(Product product, User user) {
}
}

private PageRequest createPageRequest(Integer page, Integer size) {
int pageIndex = (page == null || page < 1) ? 0 : page - 1; // 0-based로 변환
int pageSize = (size == null || size < 1) ? 20 : size;

return PageRequest.of(pageIndex, pageSize, Sort.by(Sort.Direction.ASC, "name"));
}

private Page<Product> findProductsByFilter(
Long storeId, Boolean isActivate, String nameKeyword, PageRequest pageable
) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.almang.inventory.vendor.controller;

import com.almang.inventory.global.api.ApiResponse;
import com.almang.inventory.global.api.PageResponse;
import com.almang.inventory.global.api.SuccessMessage;
import com.almang.inventory.global.security.principal.CustomUserPrincipal;
import com.almang.inventory.vendor.dto.request.CreateVendorRequest;
Expand All @@ -20,6 +21,7 @@
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@Slf4j
Expand Down Expand Up @@ -76,4 +78,24 @@ public ResponseEntity<ApiResponse<VendorResponse>> getVendorDetail(
ApiResponse.success(SuccessMessage.GET_VENDOR_DETAIL_SUCCESS.getMessage(), response)
);
}

@GetMapping
@Operation(summary = "발주처 목록 조회", description = "발주처 목록을 페이지네이션, 활성 여부, 이름 검색 조건과 함께 조회합니다.")
public ResponseEntity<ApiResponse<PageResponse<VendorResponse>>> getVendorList(
@AuthenticationPrincipal CustomUserPrincipal userPrincipal,
@RequestParam(value = "page", required = false) Integer page,
@RequestParam(value = "size", required = false) Integer size,
@RequestParam(value = "isActivate", required = false) Boolean isActivate,
@RequestParam(value = "name", required = false) String nameKeyword
) {
Long userId = userPrincipal.getId();
log.info("[VendorController] 발주처 목록 조회 요청 - userId: {}, page: {}, size: {}, isActivate: {}, name: {}",
userId, page, size, isActivate, nameKeyword);
PageResponse<VendorResponse> response =
vendorService.getVendorList(userId, page, size, isActivate, nameKeyword);

return ResponseEntity.ok(
ApiResponse.success(SuccessMessage.GET_VENDOR_LIST_SUCCESS.getMessage(), response)
);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,23 @@
package com.almang.inventory.vendor.repository;

import com.almang.inventory.vendor.domain.Vendor;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;

public interface VendorRepository extends JpaRepository<Vendor, Long> {

Page<Vendor> findAllByStoreId(Long storeId, Pageable pageable);

Page<Vendor> findAllByStoreIdAndActivatedTrue(
Long storeId, Pageable pageable
);

Page<Vendor> findAllByStoreIdAndNameContainingIgnoreCase(
Long storeId, String name, Pageable pageable
);

Page<Vendor> findAllByStoreIdAndActivatedTrueAndNameContainingIgnoreCase(
Long storeId, String name, Pageable pageable
);
Comment on lines +10 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Repository 메서드가 불완전합니다

현재 activated=true인 경우만 필터링하는 메서드가 있지만, activated=false를 조회하는 메서드가 없습니다. VendorService의 필터 로직 버그와 연관된 이슈입니다.

완전한 필터링 지원을 위해 다음 메서드들을 추가하세요:

Page<Vendor> findAllByStoreIdAndActivatedFalse(
        Long storeId, Pageable pageable
);

Page<Vendor> findAllByStoreIdAndActivatedFalseAndNameContainingIgnoreCase(
        Long storeId, String name, Pageable pageable
);

또는, Spring Data JPA Specification을 사용하여 동적 쿼리를 구성하는 것도 좋은 대안입니다:

참고: Spring Data JPA Specifications

🤖 Prompt for AI Agents
In src/main/java/com/almang/inventory/vendor/repository/VendorRepository.java
around lines 10 to 22, repository methods only support filtering activated=true
but lack counterparts for activated=false which breaks service-level filtering;
add two methods to support inactive vendors: declare Page<Vendor>
findAllByStoreIdAndActivatedFalse(Long storeId, Pageable pageable) and
Page<Vendor> findAllByStoreIdAndActivatedFalseAndNameContainingIgnoreCase(Long
storeId, String name, Pageable pageable), or alternatively refactor to use
Spring Data JPA Specifications to build dynamic queries for activated true/false
and name filtering.

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package com.almang.inventory.vendor.service;

import com.almang.inventory.global.api.PageResponse;
import com.almang.inventory.global.exception.BaseException;
import com.almang.inventory.global.exception.ErrorCode;
import com.almang.inventory.global.util.PaginationUtil;
import com.almang.inventory.store.domain.Store;
import com.almang.inventory.user.domain.User;
import com.almang.inventory.user.repository.UserRepository;
import com.almang.inventory.vendor.domain.Vendor;
Expand All @@ -11,6 +14,8 @@
import com.almang.inventory.vendor.repository.VendorRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand Down Expand Up @@ -57,6 +62,22 @@ public VendorResponse getVendorDetail(Long vendorId, Long userId) {
return VendorResponse.from(vendor);
}

@Transactional(readOnly = true)
public PageResponse<VendorResponse> getVendorList(
Long userId, Integer page, Integer size, Boolean isActivate, String nameKeyword
) {
User user = findUserById(userId);
Store store = user.getStore();

log.info("[VendorService] 발주처 목록 조회 요청 - userId: {}, storeId: {}", userId, store.getId());
PageRequest pageable = PaginationUtil.createPageRequest(page, size, "name");
Page<Vendor> vendorPage = findVendorsByFilter(store.getId(), isActivate, nameKeyword, pageable);
Page<VendorResponse> mapped = vendorPage.map(VendorResponse::from);

log.info("[VendorService] 발주처 목록 조회 성공 - userId: {}, storeId: {}", userId, store.getId());
return PageResponse.from(mapped);
}

private Vendor toEntity(CreateVendorRequest request, User user) {
return Vendor.builder()
.store(user.getStore())
Expand All @@ -83,4 +104,32 @@ private Vendor findVendorByIdAndValidateAccess(Long vendorId, User user) {

return vendor;
}

private Page<Vendor> findVendorsByFilter(
Long storeId, Boolean isActivate, String nameKeyword, PageRequest pageable
) {
boolean hasName = nameKeyword != null && !nameKeyword.isBlank();

// 필터 없음
if (isActivate == null && !hasName) {
return vendorRepository.findAllByStoreId(storeId, pageable);
}

// 활성 여부
if (isActivate != null && !hasName) {
return vendorRepository.findAllByStoreIdAndActivatedTrue(storeId, pageable);
}
Comment on lines +118 to +121
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🚨 필터 로직에 버그가 있습니다

Line 119에서 isActivate != null만 체크하고 있어, isActivate=false로 요청해도 활성화된 발주처만 조회됩니다. isActivate의 실제 값을 확인해야 합니다.

다음과 같이 수정이 필요합니다:

     // 활성 여부
-    if (isActivate != null && !hasName) {
-        return vendorRepository.findAllByStoreIdAndActivatedTrue(storeId, pageable);
+    if (isActivate != null && isActivate && !hasName) {
+        return vendorRepository.findAllByStoreIdAndActivatedTrue(storeId, pageable);
     }

또한 Line 130의 결합 필터 로직도 동일하게 수정해야 합니다:

     // 활성 여부 + 이름 검색
-    return vendorRepository.findAllByStoreIdAndActivatedTrueAndNameContainingIgnoreCase(
-            storeId, nameKeyword, pageable
-    );
+    if (isActivate) {
+        return vendorRepository.findAllByStoreIdAndActivatedTrueAndNameContainingIgnoreCase(
+                storeId, nameKeyword, pageable
+        );
+    } else {
+        return vendorRepository.findAllByStoreIdAndNameContainingIgnoreCase(
+                storeId, nameKeyword, pageable
+        );
+    }

동일한 버그가 ProductService에도 존재하는지 확인이 필요합니다.


🏁 Script executed:

#!/bin/bash
# ProductService의 필터 로직 확인
echo "=== ProductService filter logic ==="
rg -n -A5 "if \(isActivate != null" --type=java src/main/java/com/almang/inventory/product/service/ProductService.java

Length of output: 257


VendorService의 설계 의도를 확인하기 위해 추가 검증이 필요합니다.

추가 검증을 위해 VendorService 컨트롤러 레이어와 메서드 문서를 확인해야 합니다.

VendorService의 컨트롤러 엔드포인트를 확인해야 합니다. 현재까지의 분석으로는 isActivate 파라미터의 설계 의도를 완전히 파악하기 어렵습니다.


또한 학습된 내용을 참고하면, ProductService의 `isActivate` 파라미터가 false 값을 지원하지 않는다고 명시되어 있습니다. 이를 바탕으로 최종 검증을 진행하겠습니다.



**원본 리뷰 코멘트가 부정확합니다.**

학습 기록에 따르면, ProductService의 `isActivate` 파라미터는 **false 값을 지원하지 않도록 설계**되었습니다. VendorService도 동일한 설계 패턴을 따르고 있으므로:

- `isActivate == null`: 활성화 여부로 필터링하지 않음
- `isActivate == true`(null이 아닐 때): 활성화된 항목만 조회
- `isActivate == false`: 지원하지 않음

현재 코드는 `isActivate != null`을 확인하는 것이 올바른 구현입니다. 이는 버그가 아니라 **의도된 설계**입니다. Spring Data JPA의 `ActivatedTrue` 메서드 명명 규칙도 이를 뒷받침합니다.

리뷰 코멘트를 제시한 제안은 false 값을 지원한다는 잘못된 가정에 기반하고 있습니다.

[incorrect_review_comment]

---

🏁 Script executed:

```shell
#!/bin/bash
# VendorService 컨트롤러 찾기 및 getVendorList 엔드포인트 확인
echo "=== Looking for VendorController ==="
find . -name "*VendorController*" -type f

echo -e "\n=== Searching for endpoint calling getVendorList ==="
rg -n "getVendorList|isActivate" --type=java -A5 -B5 | head -100

Length of output: 10730


VendorController 엔드포인트를 확인하여 API 계약을 명확히 하겠습니다.


---

🏁 Script executed:

```shell
#!/bin/bash
# VendorController에서 getVendorList 엔드포인트 확인
echo "=== VendorController getVendorList endpoint ==="
rg -n "getVendorList|@RequestParam" -A10 -B5 git/src/main/java/com/almang/inventory/vendor/controller/VendorController.java | head -60

Length of output: 166



---

🏁 Script executed:

```shell
#!/bin/bash
# VendorController에서 getVendorList 엔드포인트 확인 (경로 수정)
echo "=== VendorController getVendorList endpoint ==="
rg -n "getVendorList|@RequestParam" -A10 -B5 src/main/java/com/almang/inventory/vendor/controller/VendorController.java | head -80

Length of output: 1236


VendorRepository의 쿼리 메서드를 확인하여 false 케이스 처리 가능성을 검증하겠습니다.


---

🏁 Script executed:

```shell
#!/bin/bash
# VendorRepository 메서드 확인
echo "=== VendorRepository methods ==="
rg -n "findAllByStoreId" src/main/java/com/almang/inventory/vendor/repository/VendorRepository.java -A2

Length of output: 510


필터 로직에 실제 버그가 있습니다 - 원본 리뷰가 정확합니다.

VendorRepository에 사용 가능한 메서드를 확인한 결과:

  • findAllByStoreId (필터 없음)
  • findAllByStoreIdAndActivatedTrue (활성화만)
  • findAllByStoreIdAndNameContainingIgnoreCase (필터 없음 + 이름)
  • findAllByStoreIdAndActivatedTrueAndNameContainingIgnoreCase (활성화 + 이름)

문제점:

  • 컨트롤러는 Boolean isActivate를 수락합니다 (true, false, null 모두 가능)
  • 서비스 라인 119-121: isActivate != null만 체크하므로, isActivate=false 전송 시에도 findAllByStoreIdAndActivatedTrue를 호출합니다
  • 결과: false와 true가 동일한 결과(활성화된 발주처만) 반환 → 데이터 무결성 문제
  • 테스트도 false 케이스를 테스트하지 않음

설계 의도:
ProductService 학습 내용에 따르면 isActivate는 false를 지원하지 않는 설계입니다. 하지만 API가 false를 수락하고 서비스에서 제대로 처리하지 않으므로 버그입니다.

수정 방안:

  1. 즉시 수정 - 원본 리뷰의 제안대로 isActivate 값을 확인:

    • 라인 119: if (isActivate != null && isActivate && !hasName)
    • 라인 130+: isActivate 값에 따른 분기 처리
  2. 근본 해결 - false를 지원하지 않으려면:

    • 컨트롤러에서 @Range 또는 커스텀 검증으로 false 값 거부
    • 또는 Boolean 대신 Enum 사용하여 API 계약 명확히 하기

원본 리뷰 코멘트의 수정 제안이 타당합니다.

🤖 Prompt for AI Agents
In src/main/java/com/almang/inventory/vendor/service/VendorService.java around
lines 118-121, the current check only tests isActivate != null causing
isActivate=false to be treated like true; change the branching to explicitly
check for true (e.g., if (isActivate != null && isActivate && !hasName)) and
update subsequent conditional branches to handle all three cases
(isActivate==true, isActivate==false, isActivate==null) combined with hasName to
call the correct repository methods (no filter, activated-only, name-only, both
activated and name); also add or update unit tests to cover the false case to
prevent regression.


// 이름 검색
if (isActivate == null) {
return vendorRepository.findAllByStoreIdAndNameContainingIgnoreCase(
storeId, nameKeyword, pageable
);
}

// 활성 여부 + 이름 검색
return vendorRepository.findAllByStoreIdAndActivatedTrueAndNameContainingIgnoreCase(
storeId, nameKeyword, pageable
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;

import com.almang.inventory.global.api.PageResponse;
import com.almang.inventory.global.api.SuccessMessage;
import com.almang.inventory.global.config.TestSecurityConfig;
import com.almang.inventory.global.exception.BaseException;
Expand Down Expand Up @@ -289,4 +290,97 @@ private UsernamePasswordAuthenticationToken auth() {
.andExpect(jsonPath("$.message").value(ErrorCode.VENDOR_NOT_FOUND.getMessage()))
.andExpect(jsonPath("$.data").doesNotExist());
}

@Test
void 발주처_목록_조회에_성공한다() throws Exception {
// given
PageResponse<VendorResponse> response = new PageResponse<>(
List.of(
new VendorResponse(1L, "테스트1", VendorChannel.KAKAO, "010-1111-1111", "메모1", true, 1L),
new VendorResponse(2L, "테스트2", VendorChannel.EMAIL, "010-2222-2222", "메모2", true, 1L)
),
1,
20,
2,
1,
true
);

when(vendorService.getVendorList(anyLong(), any(), any(), any(), any()))
.thenReturn(response);

// when & then
mockMvc.perform(get("/api/v1/vendor")
.with(authentication(auth()))
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("$.status").value(200))
.andExpect(jsonPath("$.message")
.value(SuccessMessage.GET_VENDOR_LIST_SUCCESS.getMessage()))
.andExpect(jsonPath("$.data.content").isArray())
.andExpect(jsonPath("$.data.content.length()").value(2))
.andExpect(jsonPath("$.data.page").value(1))
.andExpect(jsonPath("$.data.totalElements").value(2));
}

@Test
void 발주처_목록_조회_시_활성_필터가_적용된다() throws Exception {
// given
PageResponse<VendorResponse> response = new PageResponse<>(
List.of(
new VendorResponse(1L, "활성 발주처", VendorChannel.KAKAO, "010-3333-3333", "메모", true, 1L)
),
1,
20,
1,
1,
true
);

when(vendorService.getVendorList(anyLong(), any(), any(), any(), any()))
.thenReturn(response);

// when & then
mockMvc.perform(get("/api/v1/vendor")
.with(authentication(auth()))
.param("isActivate", "true")
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("$.status").value(200))
.andExpect(jsonPath("$.message")
.value(SuccessMessage.GET_VENDOR_LIST_SUCCESS.getMessage()))
.andExpect(jsonPath("$.data.content.length()").value(1))
.andExpect(jsonPath("$.data.content[0].activated").value(true))
.andExpect(jsonPath("$.data.content[0].name").value("활성 발주처"));
}

@Test
void 발주처_목록_조회_시_이름검색_필터가_적용된다() throws Exception {
// given
PageResponse<VendorResponse> response = new PageResponse<>(
List.of(
new VendorResponse(1L, "사과 공장", VendorChannel.KAKAO, "010-4444-4444", "메모", true, 1L)
),
1,
20,
1,
1,
true
);

when(vendorService.getVendorList(anyLong(), any(), any(), any(), any()))
.thenReturn(response);

// when & then
mockMvc.perform(get("/api/v1/vendor")
.with(authentication(auth()))
.param("name", "사과")
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("$.status").value(200))
.andExpect(jsonPath("$.message")
.value(SuccessMessage.GET_VENDOR_LIST_SUCCESS.getMessage()))
.andExpect(jsonPath("$.data.content.length()").value(1))
.andExpect(jsonPath("$.data.content[0].name").value("사과 공장"));
}
}
Loading