feat/ #20 상품 데이터 정제 #39
Conversation
✅ 테스트 결과: 모든 테스트 통과
|
코드 리뷰 보고서검토 대상 파일
🔴 높은 심각도 (CRITICAL)[ExternalClothesController.java:20] URI 경로 컨벤션 위반
[ExternalClothesController.java:28-39] API 응답 타입 위반
[ExternalClothesController.java:29] 인증 부재 및 권한 검증 누락
🟡 중간 심각도 (NEEDS_CHANGES)[NaverItemRequest.java:10-18] Request DTO 입력값 검증 누락
[ExternalClothesController.java:28-39] Query Parameter 방식의 DTO 전달 문제
[ExternalClothesService.java:37-91] @transactional 범위 최적화
[ExternalClothesService.java:38-91] colorDtos, styleDtos null 체크 부재
[ExternalClothesService.java:25-31] 매직 스트링(Magic String) 사용
[ExternalClothesService.java:349-356] containsAny() 반복 정규화로 인한 성능 낭비
🟢 낮은 심각도 (MINOR)[ExternalClothesController.java] Swagger 어노테이션 누락
[ExternalClothesService.java:342-364] 문자열 유틸 중복 구현
[ExternalClothesService.java] 메서드 길이 및 복잡도
[ClothesRepository.java] 향후 조회 쿼리 메서드 준비 권장
✅ 잘된 점
종합 판정: CRITICAL ⛔이유:
우선 조치 사항 (우선순위순):
|
✅ 테스트 결과: 모든 테스트 통과
|
코드 리뷰 결과 (네이버 외부 상품 저장 기능)분석 대상 파일 8개를 검토하여 발견한 이슈를 심각도별로 정리했습니다. 분석 대상
🔴 High1. [ExternalClothesController.java:35-57] 인증 검증 미실시
개선 방법 @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));
}임시방편으로라도 2. [ExternalClothesService.java:97-98] N+1 쿼리 문제
개선 방법 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);
}🟡 Medium3. [SaveNaverProductRequest.java] DTO 명명 컨벤션 위반 / 역할 중복
4. [SaveNaverProductRequest.java:10-18] 요청 검증 불완전record에 검증 어노테이션이 없습니다. 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() 반복 정규화
6. [ExternalClothesService.java:114-140] refineCategory() 메서드 복잡도긴 if-else 체인으로 가독성 저하. 카테고리→키워드 매핑(Map)이나 매처 객체로 구조화 권장. 7. [ClothesRepository.java] 쿼리 메서드 확장성 (선택)현재 🟢 Low8. [NaverItemRequest.java:38-43] getCleanTitle() 위치 부적절HTML 태그 제거 로직이 request DTO에 있는 것이 부자연스러움. 유틸리티 클래스로 분리 고려. 9. [SaveNaverProductResponse.java:4] Response DTO 확장성 (선택)
10. [ExternalClothesService.java] 로깅 부재상품 저장 성공/실패, style 조회 실패 추적이 어려움. SLF4J 로깅 추가 권장. 11. [ExternalClothesService.java:162-209] refineColor() 미사용 (Dead Code)48줄짜리 12. [NaverItemRequest.java] 스타일 일관성 부족
✅ 잘된 점
종합 판정: NEEDS_CHANGES
즉시 처리 필요
|
jychoi0831
left a comment
There was a problem hiding this comment.
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 저장 후 WardrobeClothes를 OwnershipStatus.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를 사용해 작성했습니다.
✅ 테스트 결과: 모든 테스트 통과
|
코드 리뷰 결과 — External Clothes (네이버 상품 연동)
🔴 CRITICAL (즉시 수정 — 빌드/런타임 차단)C1.
|
| 우선순위 | 항목 | 영향 |
|---|---|---|
| 1 | C1·C2·C3 (누락 클래스/메서드) | 빌드 불가 |
| 2 | C2 후속 (WardrobeClothes 연결 누락) |
기능 미완성 |
| 3 | C4 (공개 엔드포인트 + userId 미검증) | 보안 취약점 |
| 4 | H1~H3 | 성능/설계 |
| 5 | M·L | 가독성/유지보수 |
결론: 컴파일 자체가 불가능한 상태(C1~C3)이므로 우선 누락된 클래스·메서드를 채우고, 이어서 옷장 연결 누락(C2 후속)과 인증/소유권 검증(C4)을 반드시 해결한 뒤 배포 검토가 필요합니다.
|
|
✅ 테스트 결과: 모든 테스트 통과
|
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 13개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
There was a problem hiding this comment.
현재 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.ClothesStyleDto와 ClothingColorDto를 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:39는 getOrCreateExternalClothes(...)만 호출하고, 바로 아래 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 제목의 "유저 옷장 등록" 기능이 실제로는 동작하지 않는 상태입니다.
해결 방향:
상품 마스터를 생성/재사용한 뒤 같은 트랜잭션에서 해당 사용자의 Wardrobe와 Clothes를 연결하는 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:33은 authenticatedUserId = 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를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 13개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
There was a problem hiding this comment.
현재 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.ClothesStyleDto와 ClothingColorDto를 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:37은 getOrCreateExternalClothes(...)만 호출하고, 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에서도 완료되지 않았습니다.
해결 방향:
상품 마스터를 생성/재사용한 뒤 같은 트랜잭션에서 해당 사용자의 Wardrobe와 Clothes를 연결하는 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:33은 authenticatedUserId = 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를 사용해 작성했습니다.
|
|
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
There was a problem hiding this comment.
현재 head commit a3a14506 기준으로 확인했습니다.
P1: 네이버 상품 저장 API가 여전히 사용자 위시리스트/옷장에 연결하지 않습니다
src/main/java/com/closetnangam/be/global/external/clothes/controller/ExternalClothesController.java:37은 getOrCreateExternalClothes(...)만 호출하고, 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 제목의 핵심인 "유저 옷장 등록" 기능이 아직 완료되지 않은 상태입니다.
해결 방향:
상품 마스터를 생성/재사용한 뒤 같은 트랜잭션에서 해당 사용자의 Wardrobe와 Clothes를 연결하는 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:33은 authenticatedUserId = 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를 사용해 작성했습니다.
|
현재 head commit P2: 사용자 옷장 관련 코드와 표현은 제거하거나 분리하는 편이 좋겠습니다 영향: 이번 PR의 목표가 마스터 데이터 정제/저장까지라면, 사용자 ID와 옷장 관련 주석은 오히려 기능 범위를 헷갈리게 만듭니다. 해결 방향: 사용자 옷장 등록과 관련된 주입, 하드코딩 ID, 주석은 제거하거나 후속 PR로 명확히 분리해 주세요. 본 리뷰는 Codex를 사용해 작성했습니다. |
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
There was a problem hiding this comment.
현재 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가 납니다. 최신 develop은 migration.sql에 info_source 마이그레이션 내용을 추가했는데, 이 PR은 같은 파일을 삭제하고 있어 현재 상태로는 병합할 수 없습니다.
영향:
충돌이 해결되지 않으면 PR을 merge할 수 없고, 수동으로 잘못 해결하면 최신 develop의 info_source/registration_source 보정 안내가 사라질 수 있습니다.
해결 방향:
develop을 rebase/merge한 뒤 migration.sql 삭제 여부를 다시 결정해야 합니다. 현재 develop의 info_source 마이그레이션 안내와 migration-info-source-enum.sql 연계는 유지한 상태로 이 PR의 변경을 통합하는 쪽이 안전합니다.
P1: 최신 develop 기준으로 외부 상품 생성 시 infoSource가 빠집니다
현재 origin/develop의 Clothes 엔티티는 infoSource를 필수로 받고, 생성자에서 null이면 예외를 던집니다. 그런데 이 PR의 ExternalClothesService.java:82 신규 상품 생성 builder에는 .sourceType(SourceType.WISHLIST) 이후 .infoSource(...) 설정이 없습니다.
영향:
충돌을 해결해 최신 develop과 합쳐지면 네이버 외부 상품이 처음 저장되는 경로에서 Clothes.builder().build() 시점에 옷 정보 출처는 필수입니다. 예외가 발생할 수 있습니다. PR head 단독 compileJava/test는 통과하지만, 최신 base와 통합된 런타임 경로가 깨질 가능성이 높습니다.
해결 방향:
외부 쇼핑몰 기반 마스터 데이터라면 ClothesInfoSource.EXTERNAL_SHOPPING을 명시해 생성해야 합니다. 최신 develop의 Clothes 변경을 반영해 builder 호출부를 업데이트해 주세요.
P2: 공개 경로에서 인증 컨텍스트를 요구합니다
SecurityConfig.java:33에서 /api/v1/external/clothes/**를 permitAll 공개 경로로 열어두고 있는데, ExternalClothesController.java:32는 SecurityUtils.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:CONFLICTINGgit merge-tree --write-tree origin/develop origin/feat/#20:src/main/resources/db/migration.sqlmodify/delete conflict
본 리뷰는 Codex를 사용해 작성했습니다.
…BE5_FinalProject_Team4_BE into feat/#20 # Conflicts: # src/main/resources/db/migration.sql
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
There was a problem hiding this comment.
현재 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_image를 profile_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:17과 src/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:36의 PUBLIC_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를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
There was a problem hiding this comment.
현재 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-token이 userId 요청 파라미터를 받아 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_image를 profile_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를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 12개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
There was a problem hiding this comment.
현재 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_image를 profile_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
left a comment
There was a problem hiding this comment.
현재 head commit 0475247 기준으로 확인했습니다.
이전 리뷰에서 우려했던 공개 mock-token 이슈는 MockAuthController에 @Profile("local")이 추가되어 운영 프로파일 기준으로는 완화된 것으로 확인했습니다.
현재 프로젝트가 아직 AWS/운영/공유 DB 없이 로컬 개발 단계이고, 보존해야 하는 데이터베이스가 없는 상태라면 이번 PR에서 발견된 내용은 merge를 막을 blocker로 보지 않아도 될 것 같습니다.
Non-blocking: User 컬럼명 변경이 이번 PR 범위에 포함된 이유 확인 부탁드립니다
이번 PR의 주요 목적은 네이버 상품 데이터 정제 및 외부 상품 등록 흐름으로 이해했습니다.
그런데 함께 변경된 User 엔티티에서 사용자 PK 컬럼이 id에서 user_id로, profile_image가 profile_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를 사용해 작성했습니다.
📌 관련 이슈
Closes #20
🛠️ 작업 내용
✅ 변경 사항
🔍 테스트 내용
📷 스크린샷 (선택사항)
💬 리뷰어에게
📋 PR 체크리스트
develop브랜치를 base로 설정했나요?