Skip to content

feat/ #20 상품 데이터 정제 #39

Merged
seokminseok merged 13 commits into
developfrom
feat/#20
Jun 1, 2026
Merged

feat/ #20 상품 데이터 정제 #39
seokminseok merged 13 commits into
developfrom
feat/#20

Conversation

@seokminseok

Copy link
Copy Markdown
Collaborator

📌 관련 이슈

관련된 이슈 번호를 작성해 주세요.

Closes #20


🛠️ 작업 내용

구현한 내용을 간략히 설명해 주세요.

✅ 변경 사항

  • [ ]
  • [ ]

🔍 테스트 내용

테스트한 방법과 결과를 작성해 주세요.

  • 단위 테스트 작성 / 확인
  • 기능 동작 확인

📷 스크린샷 (선택사항)

UI 변경이 있는 경우 첨부해 주세요.

💬 리뷰어에게

리뷰 시 특별히 봐줬으면 하는 부분이 있다면 작성해 주세요.


📋 PR 체크리스트

  • develop 브랜치를 base로 설정했나요?
  • 코드 컨벤션을 준수했나요?
  • 불필요한 주석 / 디버그 코드를 제거했나요?
  • 관련 이슈 번호를 연결했나요?

@seokminseok seokminseok linked an issue May 29, 2026 that may be closed by this pull request
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown

Test Results

1 tests  ±0   1 ✅ ±0   0s ⏱️ ±0s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 0475247. ± Comparison against base commit 9aaef35.

♻️ This comment has been updated with latest results.

@github-actions

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@github-actions

Copy link
Copy Markdown

코드 리뷰 보고서

검토 대상 파일

  1. ClothesRepository.java
  2. ExternalClothesController.java
  3. NaverItemRequest.java
  4. ClothesStyleDto.java
  5. ClothingColorDto.java
  6. ExternalClothesService.java

🔴 높은 심각도 (CRITICAL)

[ExternalClothesController.java:20] URI 경로 컨벤션 위반

  • 문제: /api/external/clothes 경로는 프로젝트 컨벤션인 /api/v1 prefix가 누락되어 있습니다.
  • 이유: CLAUDE.md에 명시된 대로 모든 새로운 엔드포인트는 /api/v1로 시작해야 합니다. 버전 관리 없이는 API 진화 시 하위호환성 관리가 어려워집니다.
  • 개선 방법:
    // Before
    @RequestMapping("/api/external/clothes")
    
    // After
    @RequestMapping("/api/v1/external/clothes")

[ExternalClothesController.java:28-39] API 응답 타입 위반

  • 문제: Controller에서 ResponseEntity<String>을 반환하고 있습니다. CLAUDE.md의 아키텍처 패턴에 따르면 모든 엔드포인트는 ApiResponse<T> 래퍼를 사용해야 합니다.
  • 이유: 일관된 API 응답 구조(success, data, message)를 사용하지 않으면 클라이언트에서 처리 로직이 복잡해집니다. 현재 문자열 응답은 데이터 구조가 없어 프론트엔드 파싱이 어렵습니다.
  • 개선 방법:
    // DTO 클래스 추가
    public record SaveNaverProductResponse(Long clothesId) {}
    
    @PostMapping("/naver")
    public ApiResponse<SaveNaverProductResponse> saveNaverProduct(
            @RequestParam Long userId,
            @RequestBody NaverItemRequest request,
            @RequestParam(required = false) List<ClothingColorDto> colorDtos,
            @RequestParam(required = false) List<ClothesStyleDto> styleDtos
    ) {
        Wardrobe wardrobe = wardrobeService.getOrCreateWardrobe(userId);
        Long clothesId = externalClothesService.saveNaverToWishlist(userId, wardrobe, request, colorDtos, styleDtos);
        return ApiResponse.ok(new SaveNaverProductResponse(clothesId));
    }

[ExternalClothesController.java:29] 인증 부재 및 권한 검증 누락

  • 문제: userId@RequestParam으로 받고 있으며, 요청자의 권한 검증(자신의 옷장에만 접근 가능)이 없습니다.
  • 이유: CLAUDE.md의 "Security" 섹션에서 명시한 대로 SecurityContextHolder 통합이 미완료 상태이며, ownership verification에 대한 TODO가 필요합니다. 현재 상태에서는 다른 사용자의 userId를 입력하여 임의로 옷장에 상품을 추가할 수 있습니다(권한 우회).
  • 개선 방법:
    @PostMapping("/naver")
    public ApiResponse<SaveNaverProductResponse> saveNaverProduct(
            @RequestParam Long userId,
            @RequestBody NaverItemRequest request,
            @RequestParam(required = false) List<ClothingColorDto> colorDtos,
            @RequestParam(required = false) List<ClothesStyleDto> styleDtos
    ) {
        // TODO: SecurityContextHolder 통합 후 아래 검증 추가
        // Long authenticatedUserId = SecurityUtils.getCurrentUserId();
        // if (!userId.equals(authenticatedUserId)) {
        //     throw new IllegalArgumentException("자신의 옷장에만 접근할 수 있습니다");
        // }
    
        Wardrobe wardrobe = wardrobeService.getOrCreateWardrobe(userId);
        Long clothesId = externalClothesService.saveNaverToWishlist(userId, wardrobe, request, colorDtos, styleDtos);
        return ApiResponse.ok(new SaveNaverProductResponse(clothesId));
    }

🟡 중간 심각도 (NEEDS_CHANGES)

[NaverItemRequest.java:10-18] Request DTO 입력값 검증 누락

  • 문제: NaverItemRequest의 모든 필드가 @NotNull, @NotBlank, @Size 등의 검증 어노테이션 없이 정의되어 있습니다.
  • 이유: 요청이 들어올 때 null 값이나 빈 문자열이 그대로 통과되어 getCleanTitle() 같은 메서드에서 NullPointerException이 발생할 수 있습니다. @Valid를 Controller에 사용하려면 DTO 필드에 검증 제약이 필요합니다.
  • 개선 방법:
    @Getter
    @NoArgsConstructor
    @AllArgsConstructor
    public class NaverItemRequest {
        @NotBlank(message = "상품명은 필수입니다")
        @Size(max = 255, message = "상품명은 255자 이하여야 합니다")
        private String title;
    
        @NotBlank(message = "상품 링크는 필수입니다")
        @Size(max = 500, message = "링크는 500자 이하여야 합니다")
        private String link;
    
        @NotBlank(message = "이미지 URL은 필수입니다")
        @Size(max = 500, message = "이미지 URL은 500자 이하여야 합니다")
        private String image;
    
        @NotBlank(message = "가격은 필수입니다")
        private String lprice;
    
        @Size(max = 100) private String brand;
        @NotBlank(message = "상품 ID는 필수입니다") private String productId;
        @Size(max = 100) private String category1;
        @Size(max = 100) private String category3;
    }
    그리고 Controller의 @RequestBody@Valid 추가.

[ExternalClothesController.java:28-39] Query Parameter 방식의 DTO 전달 문제

  • 문제: ClothingColorDtoClothesStyleDto@RequestParam으로 받아지고 있습니다. JSON 배열을 쿼리 파라미터로 직렬화할 수 없어 클라이언트 구현이 매우 복잡합니다.
  • 이유: 복잡한 객체 배열은 JSON body에 @RequestBody로 전달하는 것이 REST API 컨벤션이며, 구현도 훨씬 간단합니다.
  • 개선 방법:
    public record SaveNaverProductRequest(
        @Valid NaverItemRequest naverItem,
        List<ClothingColorDto> colors,
        List<ClothesStyleDto> styles
    ) {}
    
    @PostMapping("/naver")
    public ApiResponse<SaveNaverProductResponse> saveNaverProduct(
            @RequestParam Long userId,
            @Valid @RequestBody SaveNaverProductRequest request
    ) {
        Wardrobe wardrobe = wardrobeService.getOrCreateWardrobe(userId);
        Long clothesId = externalClothesService.saveNaverToWishlist(
            userId, wardrobe, request.naverItem(), request.colors(), request.styles());
        return ApiResponse.ok(new SaveNaverProductResponse(clothesId));
    }

[ExternalClothesService.java:37-91] @transactional 범위 최적화

  • 문제: 클래스 레벨 @Transactional이 적용되어 읽기 전용 helper 메서드까지 모두 트랜잭션에 포함됩니다.
  • 이유: 쓰기 작업이 있는 메서드에만 @Transactional을 적용하고, 읽기 전용 작업은 @Transactional(readOnly = true)를 사용해야 성능상 이점(flush 비활성화 등)을 얻을 수 있습니다.
  • 개선 방법:
    @Service
    @RequiredArgsConstructor
    public class ExternalClothesService {
    
        @Transactional  // 쓰기 작업 메서드에만 적용
        public Long saveNaverToWishlist(...) { ... }
    }

[ExternalClothesService.java:38-91] colorDtos, styleDtos null 체크 부재

  • 문제: saveNaverToWishlist에서 colorDtos, styleDtos에 대한 null 체크가 없습니다.
  • 이유: Controller에서 @RequestParam(required = false)로 정의되어 null이 올 수 있는데, for 루프에서 NPE가 발생합니다.
  • 개선 방법:
    List<ClothingColorDto> colors = colorDtos != null ? colorDtos : new ArrayList<>();
    List<ClothesStyleDto> styles = styleDtos != null ? styleDtos : new ArrayList<>();

[ExternalClothesService.java:25-31] 매직 스트링(Magic String) 사용

  • 문제: 상품 분류, 색상 기본값 등이 문자열 상수로 흩어져 있습니다.
  • 이유: 같은 값이 여러 곳에서 반복되면 유지보수 시 일관성을 보장하기 어렵습니다. Enum으로 정의하면 타입 안정성을 확보할 수 있습니다.
  • 개선 방법: static final 상수로 추출하거나, ItemCategory, ItemType 등을 Enum으로 정의.

[ExternalClothesService.java:349-356] containsAny() 반복 정규화로 인한 성능 낭비

  • 문제: containsAny() 메서드가 호출될 때마다 normalizeText(keyword)를 반복 호출합니다.
  • 이유: 같은 키워드가 반복 정규화되므로 불필요한 CPU 사이클을 낭비합니다.
  • 개선 방법: 정규화된 키워드를 캐싱(ConcurrentHashMap)하거나 상수로 사전 정규화.

🟢 낮은 심각도 (MINOR)

[ExternalClothesController.java] Swagger 어노테이션 누락

  • 문제: Controller 클래스/메서드에 @Tag, @Operation이 없습니다.
  • 이유: CLAUDE.md에서 Swagger 어노테이션 적용을 명시했습니다.
  • 개선 방법: @Tag(name = "External Clothes"), @Operation(summary = ...) 추가.

[ExternalClothesService.java:342-364] 문자열 유틸 중복 구현

  • 문제: hasText(), defaultIfBlank(), normalizeText()를 직접 구현했습니다.
  • 이유: Spring의 StringUtils.hasText() 등 공통 유틸 재사용으로 DRY 원칙을 지킬 수 있습니다.
  • 개선 방법: org.springframework.util.StringUtils 사용 또는 공통 유틸 클래스 추출.

[ExternalClothesService.java] 메서드 길이 및 복잡도

  • 문제: refineCategory(), refineItemType() 등이 수백 줄의 if-else 구조입니다.
  • 이유: 길고 복잡한 메서드는 테스트와 유지보수가 어렵습니다.
  • 개선 방법: ClothingCategoryClassifier, ClothingColorDetector 등 별도 @Component로 분류 로직 추출.

[ClothesRepository.java] 향후 조회 쿼리 메서드 준비 권장

  • 문제: existsByExternalProductId() 외 비즈니스 쿼리가 없습니다.
  • 이유: 향후 옷 조회 기능 개발 시 N+1 문제를 예방하려면 @EntityGraph 기반 쿼리를 미리 준비하는 것이 좋습니다.
  • 개선 방법: @EntityGraph(attributePaths = {...})를 사용한 fetch join 쿼리 메서드 추가(향후).

✅ 잘된 점

  1. Record 클래스 활용: ClothesStyleDto, ClothingColorDto가 record로 구현되어 간결하고 불변성을 보장합니다.
  2. 입력값 검증 (부분): ClothesStyleDto, ClothingColorDto@NotNull, @PositiveOrZero, @Size, @NotBlank 등이 적용되어 있습니다.
  3. Builder 패턴 사용: Clothes.builder()로 엔티티를 생성하여 CLAUDE.md 패턴을 따릅니다.
  4. 헬퍼 메서드 분리: 카테고리/아이템 분류 로직이 메서드별로 명확한 책임을 갖습니다.
  5. 상품 중복 체크: 네이버 상품 ID로 중복 확인 후 IllegalStateException 발생으로 데이터 일관성을 유지합니다.
  6. Cascade 저장: @OneToMany(cascade = CascadeType.ALL)로 색상/스타일 태그가 자동 저장되도록 설계했습니다.

종합 판정: CRITICAL

이유:

  • 🔴 높은 심각도 3건: URI 컨벤션 위반, API 응답 래퍼 부재, 인증·권한 검증 누락
  • 🟡 중간 심각도 6건: 입력값 검증 누락, 파라미터 전달 방식 부적절, @transactional 범위 최적화, 매직 문자열, null 체크 부재, 정규화 반복 비효율

우선 조치 사항 (우선순위순):

  1. URI를 /api/v1/external/clothes로 변경
  2. API 응답을 ApiResponse<T> 래퍼로 변환
  3. 인증·권한 검증 로직 추가 (최소한 TODO 주석)
  4. NaverItemRequest에 입력값 검증 어노테이션 추가
  5. colorDtos, styleDtos에 null 체크 추가
  6. @Transactional 범위 최적화 및 @Transactional(readOnly = true) 활용

참고: CLAUDE.md의 컨벤션과 아키텍처 패턴을 일관되게 따르는 것이 팀 전체의 코드 품질과 유지보수성을 보장합니다. 특히 Security 통합과 API 응답 래퍼는 조기에 표준화할수록 후속 리팩토링 비용이 줄어듭니다.

@github-actions

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@github-actions

Copy link
Copy Markdown

코드 리뷰 결과 (네이버 외부 상품 저장 기능)

분석 대상 파일 8개를 검토하여 발견한 이슈를 심각도별로 정리했습니다.

분석 대상

  • domain/clothes/repository/ClothesRepository.java
  • global/external/clothes/controller/ExternalClothesController.java
  • global/external/clothes/dto/NaverItemRequest.java
  • global/external/clothes/dto/record/SaveNaverProductRequest.java
  • global/external/clothes/dto/request/ClothesStyleDto.java
  • global/external/clothes/dto/request/ClothingColorDto.java
  • global/external/clothes/dto/response/SaveNaverProductResponse.java
  • global/external/clothes/service/ExternalClothesService.java

🔴 High

1. [ExternalClothesController.java:35-57] 인증 검증 미실시

/api/v1/external/clothes/naver 엔드포인트가 @RequestParam Long userId로 클라이언트 입력을 그대로 신뢰합니다. SecurityContextHolder 통합 전 상태이지만, 악의적 사용자가 다른 사용자의 userId를 넘기면 타인의 옷장에 상품을 추가할 수 있습니다 (Broken Access Control).

개선 방법

@PostMapping("/naver")
public ApiResponse<SaveNaverProductResponse> saveNaverProduct(
        @Valid @RequestBody SaveNaverProductRequest request
) {
    Long authenticatedUserId = SecurityUtils.getCurrentUserId();
    Wardrobe wardrobe = wardrobeService.getOrCreateWardrobe(authenticatedUserId);
    Long clothesId = externalClothesService.saveNaverToWishlist(
            authenticatedUserId, wardrobe, request, request.colors(), request.styles());
    return ApiResponse.ok(new SaveNaverProductResponse(clothesId));
}

임시방편으로라도 userId.equals(authenticatedUserId) 검증을 추가해야 합니다.

2. [ExternalClothesService.java:97-98] N+1 쿼리 문제

saveNaverToWishlist()의 style 저장 반복문 내부에서 styleRepository.findById()를 호출합니다. style 10개 추가 시 SELECT 10번 발생.

개선 방법

List<Long> styleIds = safeStyles.stream().map(ClothesStyleDto::styleId).toList();
Map<Long, Style> styleMap = styleRepository.findAllById(styleIds).stream()
        .collect(Collectors.toMap(Style::getId, Function.identity()));

for (ClothesStyleDto dto : safeStyles) {
    Style style = styleMap.get(dto.styleId());
    if (style == null) {
        throw new IllegalArgumentException("스타일 없음: " + dto.styleId());
    }
    ClothesStyleTag styleTag = ClothesStyleTag.create(clothes, style, dto.styleRole(), dto.sortOrder());
    clothes.addStyleTag(styleTag);
}

🟡 Medium

3. [SaveNaverProductRequest.java] DTO 명명 컨벤션 위반 / 역할 중복

SaveNaverProductRequestNaverItemRequest 두 DTO의 역할 구분이 불명확합니다. CLAUDE.md 규칙(XxxCreateRequest)에 맞춰 NaverProductCreateRequest 등으로 명확히 분리 권장.

4. [SaveNaverProductRequest.java:10-18] 요청 검증 불완전

record에 검증 어노테이션이 없습니다. productId, cleanTitle, image, link는 필수이므로 @NotBlank, @Size 추가 필요. nested DTO(colors, styles)에는 @Valid 적용.

public record SaveNaverProductRequest(
        @NotBlank String productId,
        String brand,
        String category3,
        @JsonProperty("title") @NotBlank @Size(max = 255) String cleanTitle,
        @NotBlank @Size(max = 500) String image,
        @NotBlank @Size(max = 500) String link,
        @JsonProperty("colors") @Valid List<ClothingColorDto> colors,
        @JsonProperty("styles") @Valid List<ClothesStyleDto> styles
) {}

5. [ExternalClothesService.java:376-379] containsAny() 반복 정규화

containsAny()가 호출될 때마다 keyword를 normalizeText()로 정규화합니다. refine 메서드들에서 수십 번 호출되어 CPU/메모리 낭비. 키워드를 미리 정규화한 상수로 분리 권장.

6. [ExternalClothesService.java:114-140] refineCategory() 메서드 복잡도

긴 if-else 체인으로 가독성 저하. 카테고리→키워드 매핑(Map)이나 매처 객체로 구조화 권장.

7. [ClothesRepository.java] 쿼리 메서드 확장성 (선택)

현재 existsByExternalProductId()만 존재. 향후 findByExternalProductId() 등이 필요할 수 있음 (현재 필수 아님).


🟢 Low

8. [NaverItemRequest.java:38-43] getCleanTitle() 위치 부적절

HTML 태그 제거 로직이 request DTO에 있는 것이 부자연스러움. 유틸리티 클래스로 분리 고려.

9. [SaveNaverProductResponse.java:4] Response DTO 확장성 (선택)

clothesId만 반환. 향후 상품명/카테고리/이미지 등 추가 정보 반환 고려.

10. [ExternalClothesService.java] 로깅 부재

상품 저장 성공/실패, style 조회 실패 추적이 어려움. SLF4J 로깅 추가 권장.

11. [ExternalClothesService.java:162-209] refineColor() 미사용 (Dead Code)

48줄짜리 refineColor()가 정의되어 있으나 호출되지 않음. 삭제 또는 용도 명시 필요.

12. [NaverItemRequest.java] 스타일 일관성 부족

NaverItemRequest@Getter 클래스, SaveNaverProductRequest는 record로 패턴이 혼재. record로 통일 권장.


✅ 잘된 점

  1. 트랜잭션 관리: 클래스 레벨 @Transactional(readOnly = true) + 쓰기 메서드만 @Transactional override — 올바른 패턴.
  2. Cascade 설정: cascade = CascadeType.ALL, orphanRemoval = true로 색상/스타일 태그 자동 관리.
  3. Builder 패턴 준수: Clothes, ClothingColor, ClothesStyleTag 모두 Builder + private factory.
  4. null-safety: StringUtils.hasText()로 안전한 null/빈 문자열 처리.
  5. Validation: NaverItemRequest, ClothingColorDto, ClothesStyleDto에 검증 어노테이션 적용.
  6. API 응답 일관성: 모든 엔드포인트가 ApiResponse<T> 사용.
  7. Swagger 문서화: @Tag, @Operation 적용.
  8. Enum 활용: SourceType, ColorRole, StyleRole 등 타입 안전성 확보.

종합 판정: NEEDS_CHANGES

심각도 건수
🔴 High 2
🟡 Medium 5
🟢 Low 5

즉시 처리 필요

  1. ExternalClothesController.saveNaverProduct() — userId를 SecurityContextHolder에서 추출 (인증 검증)
  2. ExternalClothesService.saveNaverToWishlist() — N+1 쿼리 수정 (findAllById 활용)
  3. SaveNaverProductRequest — 필수 필드 검증 어노테이션 추가

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR #39 리뷰 결과입니다. 현재 head commit 5eb6e47 기준으로 확인했습니다.

P1: 네이버 상품을 저장해도 사용자 위시리스트/옷장에 연결되지 않습니다.

ExternalClothesController.java:46-53에서 wardrobeService.getOrCreateWardrobe(userId)로 옷장을 가져와 서비스에 넘기지만, ExternalClothesService.java:43-112에서는 wardrobe 파라미터를 전혀 사용하지 않고 clothesRepository.save(clothes)만 수행합니다. 현재 목록 조회는 WardrobeClothesRepository.findAllByUserIdAndOwnershipStatus()처럼 wardrobe_clothes 기준으로 조회하므로, 이 API로 생성된 clothes row는 사용자의 위시리스트 목록에 나타나지 않을 가능성이 큽니다.

기존 ClothesService.createWishlistClothes()처럼 Clothes 저장 후 WardrobeClothesOwnershipStatus.WISHLIST, RegistrationSource 값과 함께 생성해야 합니다. WardrobeClothes.userImageUrl, size, season의 필수값 처리도 함께 정해야 합니다.

P1: 요청 파라미터의 userId를 그대로 신뢰해 다른 사용자의 옷장에 저장할 수 있습니다.

ExternalClothesController.java:36-46은 인증 여부와 별개로 클라이언트가 보낸 @RequestParam Long userId를 그대로 사용합니다. 주석으로 검증 TODO가 남아 있지만 실제 검증이 없어, 로그인한 사용자가 다른 사용자의 userId를 넘기면 해당 사용자 기준으로 옷장을 만들거나 저장을 시도할 수 있습니다.

해결 방향은 path/query의 userId를 신뢰하지 않고 인증 주체에서 사용자 ID를 가져오거나, 최소한 인증 사용자 ID와 요청 userId가 같은지 확인하는 것입니다.

P2: @Valid가 있어도 상품 요청 본문 검증이 거의 수행되지 않습니다.

SaveNaverProductRequest.java:10-18의 record component에는 @NotBlank, @Size 같은 제약이 없고, colors/styles에도 cascade validation이 걸려 있지 않습니다. 그래서 ExternalClothesService.java:71-82에서 name, imageUrl, externalProductUrl 등 NOT NULL 컬럼에 null이 들어가 DB constraint 단계에서 실패하거나, productId가 비어도 UNKNOWN으로 저장되어 중복 체크가 우회될 수 있습니다.

productId, title, image, link는 DTO에서 먼저 거르고, colors/styles@Valid를 적용해 내부 DTO의 제약도 실제로 동작하게 하는 편이 안전합니다.

P2: 스타일 태그 저장이 요청 개수만큼 SELECT를 반복합니다.

ExternalClothesService.java:95-107에서 style DTO마다 styleRepository.findById()를 호출합니다. 스타일이 여러 개 들어오면 요청 하나에서 SELECT가 N번 발생하고, 중간 하나가 없을 때도 일부 조회 후 예외가 납니다.

기존 ClothesService.buildStyleTags()처럼 ID 목록을 한 번에 조회한 뒤 map으로 검증/매핑하면 쿼리 수와 오류 처리가 더 안정적입니다.


검증:

  • git diff --check origin/develop..origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 성공
  • ./gradlew test --offline: 실패. clean archive 환경에서 ${DB_USERNAME}이 그대로 사용되어 MySQL 접속 실패 후 Hibernate dialect 결정 실패로 contextLoads()가 실패했습니다.

본 리뷰는 Codex를 사용해 작성했습니다.

@github-actions

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 0
통과 0
실패 0
건너뜀 0

@github-actions

Copy link
Copy Markdown

코드 리뷰 결과 — External Clothes (네이버 상품 연동)

  • 리뷰 대상: 변경된 10개 Java 파일
  • 리뷰 일자: 2026-05-29
  • 종합 판정: 🔴 CRITICAL — 현재 상태로 컴파일 불가 (배포 불가)

아래 CRITICAL 항목들은 실제 소스 파일을 직접 열어 교차 검증했습니다.


🔴 CRITICAL (즉시 수정 — 빌드/런타임 차단)

C1. SaveNaverProductRequest 클래스가 존재하지 않음 → 컴파일 오류

  • 위치: ExternalClothesController.java:7, 38, 51
  • 검증 결과: dto/record/ 디렉터리에는 NaverProductCreateRequest.java 만 존재하며 SaveNaverProductRequest.java는 없음. 컨트롤러가 이를 import 하고 @RequestBody로 사용 → 컴파일 실패.
  • 권장: 별도 record를 만들기보다 이미 colors()/styles()/cleanTitle()를 모두 갖춘 NaverProductCreateRequest로 통일하는 것을 권장.
    import com.closetnangam.be.global.external.clothes.dto.record.NaverProductCreateRequest;
    ...
    public ApiResponse<SaveNaverProductResponse> saveNaverProduct(
            @Valid @RequestBody NaverProductCreateRequest request) { ... }

C2. ExternalClothesService.saveNaverToWishlist(...) 메서드 미구현 → 컴파일 오류

  • 위치: ExternalClothesController.java:48
  • 검증 결과: 컨트롤러는 saveNaverToWishlist(userId, wardrobe, request, colors, styles)(5개 인자)를 호출하지만, 서비스에는 getOrCreateExternalClothes(request, colorDtos, styleDtos)(3개 인자)만 존재. 메서드 시그니처 불일치로 컴파일 실패.
  • 권장: 컨트롤러가 의도한 "위시리스트 옷장 추가"까지 수행하도록 서비스에 메서드를 추가하거나, 호출부를 기존 메서드에 맞춰 정리.
    @Transactional
    public Long saveNaverToWishlist(Long userId, Wardrobe wardrobe,
                                    NaverProductCreateRequest request,
                                    List<ClothingColorDto> colors,
                                    List<ClothesStyleDto> styles) {
        Long clothesId = getOrCreateExternalClothes(request, colors, styles);
        // TODO: WardrobeClothes 생성하여 wardrobe에 연결 (현재 누락)
        return clothesId;
    }
    • ⚠️ 현재 getOrCreateExternalClothes는 마스터 Clothes만 생성/조회하고 사용자 옷장(WardrobeClothes)에 연결하는 로직이 없음. 주입된 wardrobeClothesRepository가 사용되지 않음 → 기능 미완성.

C3. ClothesRepository.findByExternalProductId(...) 메서드 미선언 → 컴파일 오류

  • 위치: ExternalClothesService.java:72, ClothesRepository.java
  • 검증 결과: 리포지토리에는 existsByExternalProductId만 선언됨. 서비스가 findByExternalProductId(productId)(반환 Optional<Clothes>)를 호출 → 컴파일 실패.
  • 권장:
    Optional<Clothes> findByExternalProductId(String externalProductId);

C4. 인증 없는 공개 엔드포인트로 타인 옷장 조작 가능 (권한 상승)

  • 위치: SecurityConfig.java:33 + ExternalClothesController.java:37
  • 검증 결과: PUBLIC_URLS"/api/v1/external/clothes/**"가 포함되어 미인증 접근 허용. 동시에 컨트롤러는 @RequestParam Long userId를 그대로 신뢰(검증 TODO만 존재). 임의의 userId로 누구나 옷장에 상품을 밀어넣을 수 있음.
  • 권장:
    1. SecurityConfigPUBLIC_URLS에서 /api/v1/external/clothes/** 제거(인증 필요).
    2. userId를 RequestParam이 아닌 SecurityContextHolder(인증 주체)에서 추출. JWT 통합 전이라면 최소한 소유권 검증 TODO를 차단 로직으로 승격.

🟠 HIGH (배포 전 수정 권장)

H1. 정규식 매 호출 컴파일 (성능)

  • 위치: ExternalClothesService.java:64
  • Pattern.compile("\\d{7,}")를 메서드 내부에서 매번 컴파일. 클래스 상수로 추출.
    private static final Pattern PRODUCT_CODE_PATTERN = Pattern.compile("\\d{7,}");

H2. getOrCreateExternalClothes 메서드 책임 과다 + 이름 불일치

  • 위치: ExternalClothesService.java:43
  • get이라는 이름이지만 엔티티 생성·색상/스타일 태그 저장(side effect) 수행. 조회/생성 책임 분리 또는 createOrGet...으로 개명 권장.

H3. 클래스 레벨 @Transactional(readOnly = true) 실효성

  • 위치: ExternalClothesService.java:27
  • 유일한 public 메서드가 쓰기 작업(@Transactional 오버라이드)이라 클래스 기본값(readOnly)이 오해 소지. readOnly 조회 메서드가 추가될 예정이 아니면 클래스 레벨 선언 제거 검토.

🟡 MEDIUM

M1. DTO에 비즈니스 로직 혼입 가능성

  • 위치: NaverItemRequest.java (getCleanTitle() 등 정제 로직)
  • HTML 태그/품번 정제는 Service 책임. 또한 컨트롤러는 NaverItemRequest를 사용하지 않고 주석에서만 언급 → 사용되지 않는 DTO인지 확인하고 제거 검토.

M2. cleanTitle 필드명과 JSON 매핑의 모호함

  • 위치: NaverProductCreateRequest.java:20-23
  • 클라이언트는 title을 보내는데 필드명은 cleanTitle(아직 정제 전 원본). 의미가 어긋나므로 필드명을 title로 두고 정제 결과는 서비스 내 지역변수로 다루는 것을 권장.

M3. null 가드 패턴 단순화

  • 위치: ExternalClothesService.java:49-50
  • (colorDtos != null) ? colorDtos : new ArrayList<>()colorDtos != null ? colorDtos : List.of() 또는 CollectionUtils.isEmpty(...)로 간결화.

M4. /api/categories/** 임시 공개 경로

  • 위치: SecurityConfig.java:31
  • "FE 마이그레이션 후 제거" TODO. 제거 기한을 이슈로 트래킹.

🟢 LOW

L1. 기본값 상수 네이밍

  • 위치: ExternalClothesService.java:30-36
  • DEFAULT_TOP_ITEM_TYPE 등 — "매칭 실패 시 fallback"이라는 의미를 이름이나 주석으로 명확화하면 가독성 향상.

L2. containsAny 키워드 반복 정규화 (미세 성능)

  • 위치: ExternalClothesService.java:383-393
  • 코드 내 주석대로 키워드를 사전 정규화해 상수화하면 반복 normalizeText(keyword) 호출 제거 가능. (호출 빈도상 영향은 작음)

✅ 잘된 점

  • Clothes 엔티티 생성에 Builder 패턴 사용 (CLAUDE.md 규약 준수).
  • 쓰기 메서드에 @Transactional 명시.
  • DTO에 @Valid, @NotBlank, @Size 등 입력 검증 적용.
  • 컨트롤러에 @Tag/@Operation Swagger 문서화.
  • 스타일 조회 시 findAllById + Map 변환으로 N+1 회피 (단건 처리 기준 양호).
  • normalizeText에서 Locale.ROOT 사용 — 로케일 독립 변환으로 안전.

종합 및 우선순위

우선순위 항목 영향
1 C1·C2·C3 (누락 클래스/메서드) 빌드 불가
2 C2 후속 (WardrobeClothes 연결 누락) 기능 미완성
3 C4 (공개 엔드포인트 + userId 미검증) 보안 취약점
4 H1~H3 성능/설계
5 M·L 가독성/유지보수

결론: 컴파일 자체가 불가능한 상태(C1~C3)이므로 우선 누락된 클래스·메서드를 채우고, 이어서 옷장 연결 누락(C2 후속)과 인증/소유권 검증(C4)을 반드시 해결한 뒤 배포 검토가 필요합니다.

@github-actions

Copy link
Copy Markdown

⚠️ Claude 리뷰 생성 중 문제가 발생했습니다. Actions 로그를 확인해 주세요.

@github-actions

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@github-actions

Copy link
Copy Markdown

ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 13개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다.

@github-actions

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 0
통과 0
실패 0
건너뜀 0

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit c7d6b626 기준으로 확인했습니다.

P1: global.external.naver DTO의 누락된 import 때문에 전체 빌드가 실패합니다

src/main/java/com/closetnangam/be/global/external/naver/dto/record/NaverProductCreateRequest.java:4에서 com.closetnangam.be.global.external.naver.dto.request.ClothesStyleDtoClothingColorDto를 import하지만, 이번 PR에는 해당 global.external.naver.dto.request 패키지의 DTO들이 추가되지 않았습니다. 실제로 compileJava --offline가 이 파일의 import와 record component 타입을 해석하지 못해 4개 컴파일 오류로 실패합니다.

영향:

현재 PR head는 애플리케이션이 컴파일되지 않아 병합 후 배포가 불가능합니다. global.external.clothes.dto.request 쪽 DTO를 사용하려던 의도라면 import/package를 맞추거나, 사용되지 않는 중복 global.external.naver.dto.record.NaverProductCreateRequest 파일을 제거해야 합니다.

해결 방향:

global.external.naver 패키지에 필요한 request DTO를 실제로 추가하거나, 이미 추가된 global.external.clothes.dto.request DTO로 import를 정리하세요. 현재 Naver API 컨트롤러가 이 record를 사용하지 않는 구조라면 중복 DTO 파일 자체를 삭제하는 편이 더 안전합니다.

P1: 네이버 상품 저장 API가 사용자 위시리스트/옷장에 연결하지 않습니다

src/main/java/com/closetnangam/be/global/external/clothes/controller/ExternalClothesController.java:39getOrCreateExternalClothes(...)만 호출하고, 바로 아래 Wardrobe 조회와 위시리스트 연결 코드는 주석 처리되어 있습니다. 서비스도 src/main/java/com/closetnangam/be/global/external/clothes/service/ExternalClothesService.java:75부터 Clothes 마스터 row만 조회/생성한 뒤 clothes.getId()를 반환하고, 주입된 WardrobeClothesRepository는 사용하지 않습니다.

영향:

기존 위시리스트 조회는 ClothesService.getWishlistClothes()에서 WardrobeClothesRepository.findAllByUserIdAndOwnershipStatus(..., WISHLIST)를 기준으로 읽습니다. 이 API로 저장한 상품은 wardrobe_clothes row가 없어서 사용자의 위시리스트 목록에 나타나지 않습니다. PR 제목의 "유저 옷장 등록" 기능이 실제로는 동작하지 않는 상태입니다.

해결 방향:

상품 마스터를 생성/재사용한 뒤 같은 트랜잭션에서 해당 사용자의 WardrobeClothes를 연결하는 WardrobeClothes를 저장해야 합니다. 이때 OwnershipStatus.WISHLIST, registrationSource, size, season, userImageUrl의 필수값 정책도 기존 ClothesService.createWishlistClothes() 흐름과 맞춰 정해 주세요.

P2: 인증이 필요한 쓰기 API를 공개 경로로 열고, 사용자 ID도 1L로 고정했습니다

src/main/java/com/closetnangam/be/global/config/SecurityConfig.java:33에서 /api/v1/external/clothes/**PUBLIC_URLS에 추가해 인증 없이 접근 가능하게 했습니다. 동시에 ExternalClothesController.java:33authenticatedUserId = 1L로 고정하고 실제 SecurityUtils.getCurrentUserId() 사용을 주석 처리했습니다.

영향:

현재는 위시리스트 연결이 주석 처리되어 있어 전역 Clothes 데이터만 누구나 생성할 수 있고, 위시리스트 연결을 되살리는 순간 모든 요청이 사용자 1번 옷장에 저장되는 권한/데이터 오염 문제가 됩니다. 외부 상품 저장은 사용자 개인 위시리스트에 영향을 주는 쓰기 작업이므로 공개 URL로 두기 어렵습니다.

해결 방향:

/api/v1/external/clothes/**를 공개 경로에서 제거하고, 컨트롤러에서는 요청 파라미터나 하드코딩 값 대신 인증 컨텍스트의 사용자 ID를 사용하세요. 로그인 연동 전 임시 엔드포인트가 필요하다면 배포 대상 코드에서는 비활성화하거나 테스트 프로파일로 분리하는 편이 안전합니다.


검증:

  • git diff --check origin/develop..origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 실패. global.external.naver.dto.request 패키지가 없어 NaverProductCreateRequest 컴파일이 실패합니다.
  • ./gradlew test --offline: 실패. compileJava 단계에서 동일한 오류로 중단됩니다.

본 리뷰는 Codex를 사용해 작성했습니다.

@github-actions

Copy link
Copy Markdown

ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 13개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다.

@github-actions

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 0
통과 0
실패 0
건너뜀 0

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit 7123ff18 기준으로 확인했습니다.

P1: 이전 빌드 차단 이슈가 그대로 남아 있어 compileJava가 실패합니다

src/main/java/com/closetnangam/be/global/external/naver/dto/record/NaverProductCreateRequest.java:4에서 com.closetnangam.be.global.external.naver.dto.request.ClothesStyleDtoClothingColorDto를 import하지만, 현재 PR에는 해당 패키지의 request DTO가 없습니다. global.external.clothes.dto.request 패키지에는 DTO가 있지만 global.external.naver.dto.request는 존재하지 않아, 현재 head에서도 동일한 컴파일 오류가 발생합니다.

영향:

애플리케이션이 컴파일되지 않으므로 병합 후 배포가 불가능합니다. 이전 Codex 리뷰에서 남긴 빌드 차단 finding이 아직 해결되지 않은 상태입니다.

해결 방향:

사용하지 않는 global.external.naver.dto.record.NaverProductCreateRequest를 제거하거나, 실제로 필요한 타입이라면 import/package를 존재하는 DTO 위치와 맞춰 주세요. 현재 구조상 Naver API 컨트롤러가 이 record를 사용하지 않으므로 중복 DTO를 삭제하는 쪽이 가장 작고 안전해 보입니다.

P1: 네이버 상품 저장 API가 여전히 사용자 위시리스트/옷장에 연결하지 않습니다

src/main/java/com/closetnangam/be/global/external/clothes/controller/ExternalClothesController.java:37getOrCreateExternalClothes(...)만 호출하고, Wardrobe 조회와 위시리스트 연결 코드는 ExternalClothesController.java:42부터 주석 처리되어 있습니다. 서비스도 src/main/java/com/closetnangam/be/global/external/clothes/service/ExternalClothesService.java:75 이후 Clothes 마스터 row만 조회/생성한 뒤 clothes.getId()를 반환하며, 주입된 WardrobeClothesRepository를 사용하지 않습니다.

영향:

기존 위시리스트 조회는 WardrobeClothesRepository.findAllByUserIdAndOwnershipStatus(..., WISHLIST) 기준으로 읽기 때문에, 이 API로 저장한 상품은 사용자의 위시리스트 목록에 나타나지 않습니다. PR의 핵심 기능인 "유저 옷장 등록"이 현재 head에서도 완료되지 않았습니다.

해결 방향:

상품 마스터를 생성/재사용한 뒤 같은 트랜잭션에서 해당 사용자의 WardrobeClothes를 연결하는 WardrobeClothes를 저장해야 합니다. OwnershipStatus.WISHLIST, registrationSource, size, season, userImageUrl 필수값 정책도 기존 ClothesService.createWishlistClothes() 흐름과 일관되게 정해 주세요.

P2: 인증이 필요한 쓰기 API가 여전히 공개 경로이고 사용자 ID도 1L로 고정되어 있습니다

src/main/java/com/closetnangam/be/global/config/SecurityConfig.java:33에서 /api/v1/external/clothes/**를 공개 경로로 열고 있습니다. 동시에 ExternalClothesController.java:33authenticatedUserId = 1L을 하드코딩하고, 실제 인증 사용자 조회는 사용하지 않습니다.

영향:

현재는 전역 Clothes 데이터 생성만 누구나 호출할 수 있고, 위시리스트 연결이 복구되는 순간 모든 요청이 사용자 1번 옷장 기준으로 처리될 위험이 있습니다. 외부 상품 저장은 개인 위시리스트를 변경하는 쓰기 작업이므로 공개 URL과 하드코딩 사용자 ID는 병합 전에 정리해야 합니다.

해결 방향:

/api/v1/external/clothes/**를 공개 경로에서 제거하고, 컨트롤러에서는 하드코딩 값 대신 인증 컨텍스트의 사용자 ID를 사용하세요. 로그인 연동 전 임시 호출이 필요하다면 배포 코드가 아닌 테스트 프로파일이나 별도 개발용 엔드포인트로 분리하는 편이 안전합니다.


검증:

  • git diff --check origin/develop..origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 실패. global.external.naver.dto.request 패키지가 없어 NaverProductCreateRequest 컴파일이 실패합니다.
  • ./gradlew test --offline: 실패. compileJava 단계에서 동일한 오류로 중단됩니다.

본 리뷰는 Codex를 사용해 작성했습니다.

@github-actions

Copy link
Copy Markdown

⚠️ Claude 리뷰 생성 중 문제가 발생했습니다. Actions 로그를 확인해 주세요.

@github-actions

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit a3a14506 기준으로 확인했습니다.

P1: 네이버 상품 저장 API가 여전히 사용자 위시리스트/옷장에 연결하지 않습니다

src/main/java/com/closetnangam/be/global/external/clothes/controller/ExternalClothesController.java:37getOrCreateExternalClothes(...)만 호출하고, Wardrobe 조회와 위시리스트 연결 코드는 ExternalClothesController.java:42부터 주석 처리되어 있습니다. 서비스도 src/main/java/com/closetnangam/be/global/external/clothes/service/ExternalClothesService.java:75 이후 Clothes 마스터 row만 조회/생성한 뒤 clothes.getId()를 반환하며, 주입된 WardrobeClothesRepository를 사용하지 않습니다.

영향:

기존 위시리스트 조회는 ClothesService.getWishlistClothes()에서 WardrobeClothesRepository.findAllByUserIdAndOwnershipStatus(..., WISHLIST)를 기준으로 읽습니다. 따라서 이 API로 저장한 상품은 wardrobe_clothes row가 없어 사용자의 위시리스트 목록에 나타나지 않습니다. PR 제목의 핵심인 "유저 옷장 등록" 기능이 아직 완료되지 않은 상태입니다.

해결 방향:

상품 마스터를 생성/재사용한 뒤 같은 트랜잭션에서 해당 사용자의 WardrobeClothes를 연결하는 WardrobeClothes를 저장해야 합니다. OwnershipStatus.WISHLIST, registrationSource, size, season, userImageUrl 필수값 정책도 기존 ClothesService.createWishlistClothes() 흐름과 일관되게 정해 주세요.

P2: 인증이 필요한 쓰기 API가 공개 경로이고 사용자 ID도 1L로 고정되어 있습니다

src/main/java/com/closetnangam/be/global/config/SecurityConfig.java:33에서 /api/v1/external/clothes/**를 공개 경로로 열고 있습니다. 동시에 ExternalClothesController.java:33authenticatedUserId = 1L을 하드코딩하고, 실제 인증 사용자 조회는 사용하지 않습니다.

영향:

현재는 누구나 전역 Clothes 데이터를 생성할 수 있고, 위시리스트 연결이 복구되는 순간 모든 요청이 사용자 1번 옷장 기준으로 처리될 위험이 있습니다. 외부 상품 저장은 개인 위시리스트를 변경하는 쓰기 작업이므로 공개 URL과 하드코딩 사용자 ID는 병합 전에 정리해야 합니다.

해결 방향:

/api/v1/external/clothes/**를 공개 경로에서 제거하고, 컨트롤러에서는 하드코딩 값 대신 인증 컨텍스트의 사용자 ID를 사용하세요. 로그인 연동 전 임시 호출이 필요하다면 배포 코드가 아닌 테스트 프로파일이나 별도 개발용 엔드포인트로 분리하는 편이 안전합니다.


검증:

  • git diff --check origin/develop..origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 성공
  • ./gradlew test --offline: 성공

본 리뷰는 Codex를 사용해 작성했습니다.

@jychoi0831

Copy link
Copy Markdown
Collaborator

현재 head commit a3a14506 기준으로 확인했습니다.

P2: 사용자 옷장 관련 코드와 표현은 제거하거나 분리하는 편이 좋겠습니다
ExternalClothesController.java:26에서 WardrobeService를 주입하고 있고, ExternalClothesController.java:33에서는 테스트용 사용자 ID를 하드코딩하고 있습니다. 또한 ExternalClothesController.java:42부터 사용자 옷장 등록 주석이 남아 있습니다.

영향:

이번 PR의 목표가 마스터 데이터 정제/저장까지라면, 사용자 ID와 옷장 관련 주석은 오히려 기능 범위를 헷갈리게 만듭니다.
리뷰어는 이 코드를 보고 “사용자 옷장 등록까지 의도했지만 미완성된 PR”로 해석할 가능성이 있습니다.

해결 방향:

사용자 옷장 등록과 관련된 주입, 하드코딩 ID, 주석은 제거하거나 후속 PR로 명확히 분리해 주세요.
API 이름과 설명도 “사용자 등록”보다 “외부 상품 마스터 정제/저장”에 가깝게 표현하는 것이 좋겠습니다.

본 리뷰는 Codex를 사용해 작성했습니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit 1a87682f 기준으로 확인했습니다.

P1: 현재 PR이 최신 develop과 merge conflict 상태입니다

GitHub mergeable 상태가 CONFLICTING이고, 로컬에서도 git merge-tree --write-tree origin/develop origin/feat/#20 확인 시 src/main/resources/db/migration.sql에서 modify/delete conflict가 납니다. 최신 developmigration.sqlinfo_source 마이그레이션 내용을 추가했는데, 이 PR은 같은 파일을 삭제하고 있어 현재 상태로는 병합할 수 없습니다.

영향:

충돌이 해결되지 않으면 PR을 merge할 수 없고, 수동으로 잘못 해결하면 최신 developinfo_source/registration_source 보정 안내가 사라질 수 있습니다.

해결 방향:

develop을 rebase/merge한 뒤 migration.sql 삭제 여부를 다시 결정해야 합니다. 현재 develop의 info_source 마이그레이션 안내와 migration-info-source-enum.sql 연계는 유지한 상태로 이 PR의 변경을 통합하는 쪽이 안전합니다.

P1: 최신 develop 기준으로 외부 상품 생성 시 infoSource가 빠집니다

현재 origin/developClothes 엔티티는 infoSource를 필수로 받고, 생성자에서 null이면 예외를 던집니다. 그런데 이 PR의 ExternalClothesService.java:82 신규 상품 생성 builder에는 .sourceType(SourceType.WISHLIST) 이후 .infoSource(...) 설정이 없습니다.

영향:

충돌을 해결해 최신 develop과 합쳐지면 네이버 외부 상품이 처음 저장되는 경로에서 Clothes.builder().build() 시점에 옷 정보 출처는 필수입니다. 예외가 발생할 수 있습니다. PR head 단독 compileJava/test는 통과하지만, 최신 base와 통합된 런타임 경로가 깨질 가능성이 높습니다.

해결 방향:

외부 쇼핑몰 기반 마스터 데이터라면 ClothesInfoSource.EXTERNAL_SHOPPING을 명시해 생성해야 합니다. 최신 developClothes 변경을 반영해 builder 호출부를 업데이트해 주세요.

P2: 공개 경로에서 인증 컨텍스트를 요구합니다

SecurityConfig.java:33에서 /api/v1/external/clothes/**permitAll 공개 경로로 열어두고 있는데, ExternalClothesController.java:32SecurityUtils.getCurrentUserId()를 호출합니다. 인증 없이 들어온 요청은 Spring Security에서 차단되지 않고 컨트롤러까지 도달한 뒤 IllegalStateException("인증 정보를 찾을 수 없습니다.")로 실패합니다.

영향:

인증이 필요한 API라면 기대한 401/403 흐름이 아니라 서버 예외로 실패하고, 공개 API라면 인증 컨텍스트를 요구하는 코드와 정책이 맞지 않습니다. 추가로 이번 PR의 /api/v1/auth/mock-token도 공개 경로에 포함되어 있지 않아 개발용 토큰 발급 용도라면 바로 호출하기 어렵습니다.

해결 방향:

외부 상품 저장에 인증이 필요하면 /api/v1/external/clothes/**PUBLIC_URLS에서 제거해 JWT 필터/인증 실패 처리가 먼저 동작하게 해야 합니다. 개발용 mock token이 필요하면 /api/v1/auth/mock-token만 명시적으로 공개하거나 프로파일 제한을 두는 편이 안전합니다.


검증:

  • git diff --check origin/develop..origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 성공
  • ./gradlew test --offline: 성공
  • gh pr view --json mergeable: CONFLICTING
  • git merge-tree --write-tree origin/develop origin/feat/#20: src/main/resources/db/migration.sql modify/delete conflict

본 리뷰는 Codex를 사용해 작성했습니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit 4f78f6c 기준으로 확인했습니다.

P1: users 기존 컬럼을 마이그레이션 없이 새 이름으로 바꿔 기존 DB가 깨질 수 있습니다

src/main/java/com/closetnangam/be/domain/user/entity/User.java:17에서 사용자 PK 컬럼을 기본 id에서 user_id로, src/main/java/com/closetnangam/be/domain/user/entity/User.java:23에서 profile_imageprofile_image_url로 변경했습니다. 그런데 이번 PR에는 기존 users.id/users.profile_image를 rename 또는 backfill하는 SQL이 없습니다.

영향:

현재 설정은 ddl-auto: update라서 컬럼 rename을 안전하게 처리해 주지 않습니다. 기존 MySQL 테이블에 users.id가 있는 상태에서 앱을 올리면 Hibernate가 새 user_id/profile_image_url 컬럼을 별도 컬럼으로 보거나, PK/auto increment 변경을 적용하지 못해 기동 또는 조회가 깨질 수 있습니다. 특히 user_accounts.user_id, wardrobes.user_id, clothing_ai_photos.user_id 같은 FK들은 기존 users.id 값을 참조하던 상태라, User 엔티티의 PK 컬럼명만 바꾸면 기존 사용자/옷장/OAuth 연동 데이터가 분리되거나 조회되지 않을 위험이 큽니다. 프로필 이미지도 기존 profile_image 값이 새 profile_image_url로 자동 이전되지 않아 기존 사용자 응답에서 사라질 수 있습니다.

해결 방향:

컬럼명을 바꿀 필요가 없다면 User 매핑 변경을 되돌리는 편이 가장 안전합니다. 실제 DB 스키마를 user_id/profile_image_url로 표준화하려는 의도라면, 앱 배포 전에 실행할 명시적 마이그레이션으로 PK/FK 제약, 컬럼 rename, 기존 데이터 backfill을 함께 처리하고 로컬/운영 DB에서 검증해야 합니다.

P2: 새 mock-token API가 인증이 필요해서 최초 토큰 발급 용도로 동작하지 않습니다

src/main/java/com/closetnangam/be/global/common/controller/MockAuthController.java:17src/main/java/com/closetnangam/be/global/common/controller/MockAuthController.java:27에서 /api/v1/auth/mock-token을 추가했지만, src/main/java/com/closetnangam/be/global/config/SecurityConfig.java:22부터 src/main/java/com/closetnangam/be/global/config/SecurityConfig.java:36PUBLIC_URLS에는 이 경로가 없습니다. 따라서 Spring Security의 .anyRequest().authenticated()에 걸려 토큰이 없는 클라이언트는 컨트롤러에 도달하지 못합니다.

영향:

이 API의 목적이 "로그인 없이 임시 액세스 토큰 발급"이라면 현재 상태로는 첫 토큰을 받을 방법이 없어 기능이 동작하지 않습니다. 반대로 운영 코드에 공개되면 안 되는 개발용 API라면, 지금처럼 일반 컨트롤러로 추가하는 것 자체가 배포 프로파일에서 혼동을 만들 수 있습니다.

해결 방향:

개발용으로 유지할 경우 /api/v1/auth/mock-token만 명시적으로 permitAll 처리하되 local/dev 프로파일로 제한하세요. 운영 배포 대상이 아니라면 컨트롤러를 제거하거나 테스트/개발 프로파일 전용 빈으로 분리하는 편이 안전합니다.


검증:

  • git diff --check origin/develop...HEAD: 통과
  • git merge-tree --write-tree origin/develop origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 성공
  • ./gradlew test --offline: 성공
  • gh pr view --json mergeable: MERGEABLE

본 리뷰는 Codex를 사용해 작성했습니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit e98b928 기준으로 확인했습니다.

P1: 공개 mock-token API로 임의 사용자 JWT를 발급할 수 있습니다

src/main/java/com/closetnangam/be/global/config/SecurityConfig.java:22부터 PUBLIC_URLS/api/v1/auth/mock-token이 포함되어 있고, SecurityConfig.java:49에서 이 목록을 permitAll()로 열고 있습니다. 동시에 src/main/java/com/closetnangam/be/global/common/controller/MockAuthController.java:27부터 GET /api/v1/auth/mock-tokenuserId 요청 파라미터를 받아 MockAuthController.java:32에서 그대로 jwtTokenProvider.createAccessToken(userId)를 호출합니다. 현재 코드에는 @Profile 같은 환경 제한이나 userId 검증이 없습니다.

영향:

배포 환경에 이 코드가 올라가면 인증되지 않은 클라이언트가 원하는 userId로 액세스 토큰을 만들 수 있습니다. 이후 JWT 필터는 토큰의 사용자 ID를 인증 주체로 신뢰하므로, 다른 사용자의 옷장/구매내역/사진 등 인증 API에 접근하는 권한 우회로 이어질 수 있습니다.

해결 방향:

운영 배포 대상에서는 이 컨트롤러를 제거하거나 local/dev/test 프로파일 전용으로 제한해야 합니다. 개발용으로 유지하더라도 공개 엔드포인트에서 임의 userId를 받는 방식은 피하고, 최소한 배포 프로파일에서 빈이 생성되지 않는지와 SecurityConfig 공개 경로가 함께 검증되도록 정리해 주세요.

P1: users 기존 컬럼을 마이그레이션 없이 새 이름으로 바꿔 기존 DB가 깨질 수 있습니다

src/main/java/com/closetnangam/be/domain/user/entity/User.java:17에서 사용자 PK 컬럼을 기본 id에서 user_id로, User.java:23에서 profile_imageprofile_image_url로 변경했습니다. 하지만 이번 PR에는 기존 users.id/users.profile_image를 rename 또는 backfill하는 리소스/마이그레이션 변경이 없습니다.

영향:

현재 설정의 ddl-auto: update는 컬럼 rename을 안전하게 처리하지 않습니다. 기존 MySQL 테이블에 users.id가 있는 상태에서 앱을 올리면 Hibernate가 user_id/profile_image_url을 별도 컬럼으로 보거나 PK/auto increment 변경을 적용하지 못해 기동 또는 조회가 깨질 수 있습니다. 또한 user_accounts.user_id, wardrobes.user_id, clothing_ai_photos.user_id 같은 FK가 기존 users.id 값을 참조하던 상태라, 엔티티의 PK 컬럼명만 바꾸면 기존 사용자/옷장/OAuth 연동 데이터가 분리되거나 조회되지 않을 위험이 큽니다. 프로필 이미지도 기존 profile_image 값이 새 컬럼으로 자동 이전되지 않습니다.

해결 방향:

컬럼명을 바꿀 필요가 없다면 User 매핑 변경을 되돌리는 편이 가장 안전합니다. 실제 DB 스키마를 user_id/profile_image_url로 표준화하려는 의도라면, 앱 배포 전에 실행할 명시적 마이그레이션으로 PK/FK 제약, 컬럼 rename, 기존 데이터 backfill을 함께 처리하고 로컬/운영 DB에서 검증해야 합니다.


검증:

  • git diff --check origin/develop...HEAD: 통과
  • git merge-tree --write-tree origin/develop origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 성공
  • ./gradlew test --offline: 성공
  • gh pr view --json mergeable: MERGEABLE

본 리뷰는 Codex를 사용해 작성했습니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

✅ 테스트 결과: 모든 테스트 통과

항목
전체 1
통과 1
실패 0
건너뜀 0

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit 0475247 기준으로 확인했습니다.

이전 리뷰의 공개 mock-token 이슈는 MockAuthController@Profile("local")이 추가되어 운영 프로파일 기준으로는 완화된 것으로 확인했습니다. 다만 아래 blocker가 여전히 남아 있습니다.

P1: users 기존 컬럼을 마이그레이션 없이 새 이름으로 바꿔 기존 DB가 깨질 수 있습니다

src/main/java/com/closetnangam/be/domain/user/entity/User.java:17에서 사용자 PK 컬럼을 기본 id에서 user_id로, User.java:23에서 profile_imageprofile_image_url로 변경했습니다. 하지만 이번 PR에는 기존 users.id/users.profile_image를 rename 또는 backfill하는 리소스/마이그레이션 변경이 없습니다. 현재 설정도 src/main/resources/application.yml:15에서 ddl-auto: update를 사용하고 있어 Hibernate가 안전한 컬럼 rename을 대신 처리해 주지 않습니다.

영향:

기존 MySQL 테이블에 users.id가 있는 상태에서 앱을 올리면 Hibernate가 user_id/profile_image_url을 별도 컬럼으로 보거나 PK/auto increment 변경을 적용하지 못해 기동 또는 조회가 깨질 수 있습니다. 또한 user_accounts.user_id, wardrobes.user_id, clothing_ai_photos.user_id 같은 FK가 기존 users.id 값을 참조하던 상태라, 엔티티의 PK 컬럼명만 바꾸면 기존 사용자/옷장/OAuth 연동 데이터가 분리되거나 조회되지 않을 위험이 큽니다. 프로필 이미지도 기존 profile_image 값이 새 컬럼으로 자동 이전되지 않습니다.

해결 방향:

컬럼명을 바꿀 필요가 없다면 User 매핑 변경을 되돌리는 편이 가장 안전합니다. 실제 DB 스키마를 user_id/profile_image_url로 표준화하려는 의도라면, 앱 배포 전에 실행할 명시적 마이그레이션으로 PK/FK 제약, 컬럼 rename, 기존 데이터 backfill을 함께 처리하고 로컬/운영 DB에서 검증해야 합니다.


검증:

  • git diff --check origin/develop...HEAD: 통과
  • git merge-tree --write-tree origin/develop origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 성공
  • ./gradlew test --offline: 성공
  • gh pr view --json mergeable: MERGEABLE

본 리뷰는 Codex를 사용해 작성했습니다.

@jychoi0831 jychoi0831 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

현재 head commit 0475247 기준으로 확인했습니다.

이전 리뷰에서 우려했던 공개 mock-token 이슈는 MockAuthController@Profile("local")이 추가되어 운영 프로파일 기준으로는 완화된 것으로 확인했습니다.

현재 프로젝트가 아직 AWS/운영/공유 DB 없이 로컬 개발 단계이고, 보존해야 하는 데이터베이스가 없는 상태라면 이번 PR에서 발견된 내용은 merge를 막을 blocker로 보지 않아도 될 것 같습니다.

Non-blocking: User 컬럼명 변경이 이번 PR 범위에 포함된 이유 확인 부탁드립니다

이번 PR의 주요 목적은 네이버 상품 데이터 정제 및 외부 상품 등록 흐름으로 이해했습니다.

그런데 함께 변경된 User 엔티티에서 사용자 PK 컬럼이 id에서 user_id로, profile_imageprofile_image_url로 변경되어 있습니다.

현재는 운영/공유 DB가 없는 로컬 개발 단계이므로 마이그레이션 누락을 blocker로 보지는 않겠습니다. 다만 PR 목적과 직접 관련된 변경은 아닌 것으로 보여, 아래 중 어떤 의도인지 확인되면 좋겠습니다.

  • 최신 ERD에 맞춰 User 테이블 컬럼명을 정리하려는 변경인지
  • 다른 기능 구현 중 필요한 변경인지
  • 실수로 포함된 변경이라 별도 PR로 분리하거나 제외해도 되는지

향후 운영/공유 DB가 생긴 뒤에는 이런 컬럼명 변경이 기존 데이터와 FK에 영향을 줄 수 있으므로, 그 시점에는 명시적인 마이그레이션 정책이 필요해 보입니다.


검증:

  • git diff --check origin/develop...HEAD: 통과
  • git merge-tree --write-tree origin/develop origin/feat/#20: 통과
  • ./gradlew compileJava --offline: 성공
  • ./gradlew test --offline: 성공
  • gh pr view --json mergeable: MERGEABLE

본 리뷰는 Codex를 사용해 작성했습니다.

@seokminseok seokminseok merged commit f3d7104 into develop Jun 1, 2026
3 checks passed
@seokminseok seokminseok deleted the feat/#20 branch June 1, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat/ 데이터 정제 후 등록

2 participants