refactor: 이미지 롤백 메서드 추가 #56
Conversation
Walkthrough이미지 업로드가 단일 파일 처리에서 다중 파일(batch) 처리로 전환되었습니다. 컨트롤러는 단일 엔드포인트에서도 파일을 1요소 배열로 감싸 서비스에 위임하고, 서비스는 배열을 순회해 업로드 결과 리스트를 반환하며 오류 시 업로드된 항목을 롤백합니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as ImageController
participant Service as ImageService
participant CDN as Cloudinary
Client->>Controller: 업로드 요청 (단일/다중)
Controller->>Service: uploadImage(MultipartFile[])
rect rgba(230,245,255,0.6)
loop 각 이미지
Service->>Service: 유효성 검사(null/empty/size/type)
Service->>CDN: 업로드(folder, public_id)
CDN-->>Service: 업로드 결과(secure_url, public_id)
Service->>Service: 응답 리스트에 추가 및 public_id 저장
end
end
alt 성공
Service-->>Controller: List<ImageSavedSuccessResponse>
note over Controller: 단일 엔드포인트는 .get(0)으로 첫 요소 반환
Controller-->>Client: 성공 응답
else 예외 발생
Service->>Service: rollbackImage(publicIds)
Service-->>Controller: 예외 전파
Controller-->>Client: 오류 응답
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/main/java/com/uhdyl/backend/image/controller/ImageController.java (1)
67-68: 더 간결한 배열 변환을 사용하세요.
List<MultipartFile>을 배열로 변환하는 더 간결한 방법이 있습니다.다음과 같이 수정하세요:
- MultipartFile[] imageArray = new MultipartFile[images.size()]; - images.toArray(imageArray); + MultipartFile[] imageArray = images.toArray(new MultipartFile[0]);src/main/java/com/uhdyl/backend/image/service/ImageService.java (4)
168-181: 외부 try-catch 블록이 불필요합니다.내부 try-catch 블록이 이미 모든 예외를 처리하고 있으므로 외부 try-catch 블록은 실제로 동작하지 않습니다.
다음과 같이 수정하세요:
public void rollbackImage(List<String> publicIds){ - try{ - for (String publicId : publicIds) { - try { - cloudinary.uploader().destroy(publicId, ObjectUtils.emptyMap()); - } catch (IOException e) { - log.warn("이미지 롤백에 실패했습니다. publicId : {}", publicId); - } + for (String publicId : publicIds) { + try { + cloudinary.uploader().destroy(publicId, ObjectUtils.emptyMap()); + } catch (IOException e) { + log.warn("이미지 롤백에 실패했습니다. publicId : {}", publicId); } } - catch (Exception e){ - log.warn("이미지 롤백이 일부 실패했습니다."); - } }
45-84: 트랜잭션 관리가 필요합니다.이미지 업로드는 외부 시스템(Cloudinary)과 상호작용하지만 트랜잭션 어노테이션이 없습니다. 데이터베이스 작업이 포함될 경우를 대비해 명시적으로 트랜잭션을 관리하는 것이 좋습니다.
다음과 같이 수정하세요:
+@Transactional public List<ImageSavedSuccessResponse> uploadImage(MultipartFile[] images, String folderPath){
174-174: 로그 레벨과 메시지를 개선하세요.롤백 실패는 데이터 일관성에 영향을 줄 수 있으므로 ERROR 레벨이 더 적절하며, 예외 정보도 로그에 포함해야 합니다.
다음과 같이 수정하세요:
- log.warn("이미지 롤백에 실패했습니다. publicId : {}", publicId); + log.error("이미지 롤백에 실패했습니다. publicId : {}", publicId, e);
51-73: 병렬 처리로 성능을 개선할 수 있습니다.여러 이미지를 순차적으로 업로드하면 시간이 오래 걸릴 수 있습니다. CompletableFuture를 사용한 병렬 처리를 고려해보세요.
대량의 이미지 업로드가 예상된다면, 다음과 같은 병렬 처리 구현을 고려하세요:
- CompletableFuture를 사용한 비동기 업로드
- 업로드 제한 시간(timeout) 설정
- 진행 상황 추적을 위한 이벤트 발행
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/uhdyl/backend/image/controller/ImageController.java(4 hunks)src/main/java/com/uhdyl/backend/image/service/ImageService.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
src/main/java/com/uhdyl/backend/image/service/ImageService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/uhdyl/backend/image/service/ImageService.java (1)
148-152: PRODUCT_IMAGE 권한 검증 누락 — userId 기반 검증으로 통일 필요ImageService의 PRODUCT_IMAGE 분기에서 imageRepository.findByImageUrl(imageUrl)로 사용자 검증이 빠져 있습니다. 레포지토리에는 이미 사용자 기반 검증 메서드 existsByUserIdAndPublicId(Long, String)가 정의·구현되어 있으므로(src/main/java/com/uhdyl/backend/image/repository/CustomImageRepository.java, src/main/java/com/uhdyl/backend/image/repository/CustomImageRepositoryImpl.java) 해당 분기에서 imageRepository.existsByUserIdAndPublicId(userId, publicId)로 권한을 확인한 뒤 publicId를 반환하거나, 대안으로 ImageRepository에 findByUser_IdAndImageUrl(Long, String) 메서드를 추가해 검증을 일관화하세요. 문제 위치: src/main/java/com/uhdyl/backend/image/service/ImageService.java (PRODUCT_IMAGE 분기).
♻️ Duplicate comments (1)
src/main/java/com/uhdyl/backend/image/service/ImageService.java (1)
68-73: 이전 NPE 지적 건(secure_url/public_id null) 선조치 확인null 체크가 선행되도록 수정되어 NPE 위험이 해소되었습니다. 좋습니다.
🧹 Nitpick comments (5)
src/main/java/com/uhdyl/backend/image/service/ImageService.java (5)
45-49: 배열 null/빈값 및 folderPath 선검증 추가 권고
images가 null/빈 배열이면 for-each에서 NPE가 나고,folderPath가 null/blank면Map.of에서 NPE가 납니다. 업로드 시작 전에 빠르게 검증해 실패 원인을 명확히 해주세요.적용 예시:
- try { - for (MultipartFile image : images) { + try { + if (images == null || images.length == 0) + throw new BusinessException(ExceptionType.INVALID_IMAGE_FILE); + if (folderPath == null || folderPath.isBlank()) { + log.warn("folderPath가 비어있습니다."); + throw new BusinessException(ExceptionType.IMAGE_UPLOAD_FAILED); + } + for (MultipartFile image : images) {
59-66: Cloudinary 응답 Map 제네릭 명시 및 중복 접근 제거로컬 변수로 꺼내어 null 체크와 사용을 일관화하면 가독성과 타입 안정성이 개선됩니다.
적용 예시:
- Map<?, ?> result = cloudinary.uploader().upload( + Map<String, Object> result = cloudinary.uploader().upload( image.getBytes(), Map.of( "folder", folderPath, "public_id", UUID.randomUUID().toString() ) ); - if (result.get("secure_url") == null || result.get("public_id") == null) + String secureUrl = (String) result.get("secure_url"); + String publicId = (String) result.get("public_id"); + if (secureUrl == null || publicId == null) throw new BusinessException(ExceptionType.IMAGE_UPLOAD_FAILED); - uploadedImagesPublicId.add(result.get("public_id").toString()); - responses.add(ImageSavedSuccessResponse.to(result.get("secure_url").toString(), result.get("public_id").toString())); + uploadedImagesPublicId.add(publicId); + responses.add(ImageSavedSuccessResponse.to(secureUrl, publicId));Also applies to: 68-73
62-63: folderPath 입력 위생 처리 권고임의 입력이 그대로 Cloudinary 폴더에 반영되므로,
..등 비정상 경로/문자 차단을 권장합니다(허용 문자 정규식).적용 스니펫(검증 위치는 선검증 구간):
// 예: 영문/숫자/슬래시/언더스코어/하이픈만 허용 if (!folderPath.matches("^[A-Za-z0-9/_-]+$")) { log.warn("허용되지 않은 folderPath: {}", folderPath); throw new BusinessException(ExceptionType.IMAGE_UPLOAD_FAILED); }
76-83: 예외 로깅 보강: 원인 파악 위해 스택트레이스 기록롤백만 하고 예외 원인이 로그에 남지 않습니다. 운영 문제 분석을 위해 로깅을 추가하세요.
적용 예시:
catch (BusinessException e) { rollbackImage(uploadedImagesPublicId); - throw e; + log.warn("이미지 업로드 중 비즈니스 예외 발생, 롤백 수행: {}", e.getMessage(), e); + throw e; } catch (Exception e) { rollbackImage(uploadedImagesPublicId); - throw new BusinessException(ExceptionType.IMAGE_UPLOAD_FAILED); + log.error("이미지 업로드 실패. 롤백 수행.", e); + throw new BusinessException(ExceptionType.IMAGE_UPLOAD_FAILED); }
169-182: 롤백 로깅에 예외 객체 포함현재는 publicId만 남겨 원인 추적이 어렵습니다. 예외 객체를 함께 로그로 남겨주세요.
적용 예시:
- } catch (IOException e) { - log.warn("이미지 롤백에 실패했습니다. publicId : {}", publicId); + } catch (IOException e) { + log.warn("이미지 롤백에 실패했습니다. publicId: {}", publicId, e); } } - } - catch (Exception e){ - log.warn("이미지 롤백이 일부 실패했습니다."); + } catch (Exception e){ + log.warn("이미지 롤백이 일부 실패했습니다.", e); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/uhdyl/backend/image/service/ImageService.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-20T04:12:43.638Z
Learnt from: kon28289
PR: UhDyL/UhDyL-Backend#43
File: src/main/java/com/uhdyl/backend/image/repository/ImageRepository.java:12-12
Timestamp: 2025-08-20T04:12:43.638Z
Learning: ImageService의 getPublicId 메서드에서 PRODUCT_IMAGE 케이스는 findByImageUrl만 사용하여 권한 검증이 누락되어 있지만, CHAT_IMAGE와 REVIEW_IMAGE는 findByUser_IdAndImageUrl을 사용하여 올바르게 권한 검증을 수행하고 있음. 일관성과 보안을 위해 모든 이미지 타입에서 사용자 ID 기반 검증이 필요함.
Applied to files:
src/main/java/com/uhdyl/backend/image/service/ImageService.java
🔇 Additional comments (1)
src/main/java/com/uhdyl/backend/image/service/ImageService.java (1)
45-45: 컨트롤러 호출부 및 빈 리스트 안전성 확인 필요
- uploadImage 시그니처는 MultipartFile[]로 변경되어 호출부들이 업데이트된 것이 확인됨 (확인된 호출: src/main/java/com/uhdyl/backend/image/controller/ImageController.java — profile:40, chat:55, product:69, review:81).
- profile/chat/review 엔드포인트에서 imageService.uploadImage(...).get(0) 사용 확인. uploadImage가 빈 리스트를 반환할 경우 IndexOutOfBoundsException 가능성 있음 — 서비스가 최소 1개 반환을 보장하거나 호출부에서 리스트 비어있음 검사 추가.
public List<ImageSavedSuccessResponse> uploadImage(MultipartFile[] images, String folderPath){
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/uhdyl/backend/image/service/ImageService.java (1)
152-156: PRODUCT_IMAGE 권한 검증 누락 — 사용자 ID 기반 검증 필요이전 리뷰에서도 동일 지적이 있었습니다.
imageUrl만으로 조회하면 타 사용자의 이미지에 접근/조회가 가능합니다. 사용자 ID를 포함한 조회로 교체하세요.아래처럼 수정 권장(레포지토리 메서드 존재 시):
- Image image = imageRepository.findByImageUrl(imageUrl) - .orElseThrow(() -> new BusinessException(ExceptionType.IMAGE_ACCESS_DENIED)); + Image image = imageRepository.findByUserIdAndImageUrl(userId, imageUrl) + .orElseThrow(() -> new BusinessException(ExceptionType.IMAGE_ACCESS_DENIED)); return ImagePublicIdResponse.to(image.getPublicId());레포지토리에 메서드가 없다면 추가가 필요합니다.
🧹 Nitpick comments (8)
src/main/java/com/uhdyl/backend/image/service/ImageService.java (8)
42-44: MIME 허용 리스트 도입 👍 + 업로드 최대 크기 상수화 권장하드코딩된 바이트 수를 상수로 추출해 재사용성을 높이세요.
아래처럼 상수를 선언하고 사용해 주세요:
private static final Set<String> ALLOWED_CONTENT_TYPES = Set.of("image/jpeg", "image/png", "image/webp", "image/gif"); + +private static final long MAX_IMAGE_BYTES = 5L * 1024 * 1024;
45-55: 배열 null/빈 배열 및 폴더 경로 검증 추가 필요
images가 null/빈 배열이면 현재 for-each에서 NPE/무효 동작 위험.folderPath도 null/빈 문자열 방지하세요.아래처럼 초기에 방어 코드를 추가하세요:
public List<ImageSavedSuccessResponse> uploadImage(MultipartFile[] images, String folderPath){ List<String> uploadedImagesPublicId = new ArrayList<>(); List<ImageSavedSuccessResponse> responses = new ArrayList<>(); try { + if (images == null || images.length == 0) + throw new BusinessException(ExceptionType.INVALID_IMAGE_FILE); + if (folderPath == null || folderPath.isBlank()) + throw new BusinessException(ExceptionType.INVALID_IMAGE_FILE); for (MultipartFile image : images) { if (image == null || image.isEmpty()) throw new BusinessException(ExceptionType.INVALID_IMAGE_FILE);
56-57: 하드코딩된 크기 비교를 상수로 교체위에서 제안한
MAX_IMAGE_BYTES사용으로 가독성/일관성 개선.- if (image.getSize() > 1024 * 1024 * 5) + if (image.getSize() > MAX_IMAGE_BYTES) throw new BusinessException(ExceptionType.IMAGE_SIZE_EXCEEDED);
59-62: 콘텐츠 타입 검증 OK — 범위 확장은 선택
contains(null)이 false이므로 null도 차단됨. 필요 시image/jpg,image/heic등 지원 포맷을 정책에 맞춰 추가 고려.
80-87: 예외 원인 로깅 추가로 트러블슈팅 용이성 향상롤백만 하고 원인은 로그에 남지 않습니다. 스택트레이스를 로그에 포함하세요.
- catch (BusinessException e) { - rollbackImage(uploadedImagesPublicId); - throw e; - } - catch (Exception e) { - rollbackImage(uploadedImagesPublicId); - throw new BusinessException(ExceptionType.IMAGE_UPLOAD_FAILED); - } + catch (BusinessException e) { + log.warn("이미지 업로드 중 비즈니스 예외 발생 — 롤백 수행", e); + rollbackImage(uploadedImagesPublicId); + throw e; + } + catch (Exception e) { + log.error("이미지 업로드 실패 — 롤백 수행", e); + rollbackImage(uploadedImagesPublicId); + throw new BusinessException(ExceptionType.IMAGE_UPLOAD_FAILED); + }
94-137: 배치 삭제의 원자성 문제 — 일부 실패 시 DB/클라우드 상태 불일치 가능한 항목에서 예외가 나면 전체 트랜잭션이 롤백되어 이전 DB 삭제도 되돌려지지만, 이미 실행된 Cloudinary 삭제는 복구 불가여서 불일치가 발생할 수 있습니다. 항목별로 독립 처리(예: 항목별 try/catch, 결과 집계 반환), 혹은 비동기 보상(아웃박스/재시도 큐)을 검토하세요.
원한다면 항목별 결과를 반환하는 시그니처/DTO로의 개편안을 제안하겠습니다.
170-171: 도달 불가능한 반환 제거 권장switch는 모든 enum 케이스를 처리합니다. 마지막
return "이미지가 존재하지 않습니다."는 실행되지 않습니다.- return ImagePublicIdResponse.to("이미지가 존재하지 않습니다."); + // 모든 케이스 처리됨
173-186: 롤백 보강: 빈 입력 처리, 예외 로그에 원인 포함, CDN 무효화 옵션안전성과 가시성을 높이세요.
public void rollbackImage(List<String> publicIds){ - try{ + try{ + if (publicIds == null || publicIds.isEmpty()) return; for (String publicId : publicIds) { try { - cloudinary.uploader().destroy(publicId, ObjectUtils.emptyMap()); - } catch (IOException e) { - log.warn("이미지 롤백에 실패했습니다. publicId : {}", publicId); + cloudinary.uploader().destroy(publicId, Map.of("invalidate", true)); + } catch (IOException e) { + log.warn("이미지 롤백에 실패했습니다. publicId: {}", publicId, e); } } } - catch (Exception e){ - log.warn("이미지 롤백이 일부 실패했습니다."); + catch (Exception e){ + log.warn("이미지 롤백이 일부 실패했습니다.", e); } }추가로, 대량 삭제가 잦다면 Admin API의 batch 삭제(
api().deleteResources) 도입을 검토하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/uhdyl/backend/image/service/ImageService.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-20T04:12:43.638Z
Learnt from: kon28289
PR: UhDyL/UhDyL-Backend#43
File: src/main/java/com/uhdyl/backend/image/repository/ImageRepository.java:12-12
Timestamp: 2025-08-20T04:12:43.638Z
Learning: ImageService의 getPublicId 메서드에서 PRODUCT_IMAGE 케이스는 findByImageUrl만 사용하여 권한 검증이 누락되어 있지만, CHAT_IMAGE와 REVIEW_IMAGE는 findByUser_IdAndImageUrl을 사용하여 올바르게 권한 검증을 수행하고 있음. 일관성과 보안을 위해 모든 이미지 타입에서 사용자 ID 기반 검증이 필요함.
Applied to files:
src/main/java/com/uhdyl/backend/image/service/ImageService.java
🔇 Additional comments (2)
src/main/java/com/uhdyl/backend/image/service/ImageService.java (2)
21-31: Slf4j 추가 적절롤백 경고 로그에 활용되고 있어 타당합니다.
45-45: 해결 — 호출부 전수 확인 완료 (문제 없음)ImageService 시그니처 변경에 대해 컨트롤러/API 호출부가 배열/리스트 변환 및 반환 처리로 모두 일치함을 확인했습니다.
- src/main/java/com/uhdyl/backend/image/service/ImageService.java:45
- src/main/java/com/uhdyl/backend/image/controller/ImageController.java:40, 55, 69, 81 (단건 업로드는 배열로 래핑 후 .get(0) 사용)
- src/main/java/com/uhdyl/backend/image/controller/ImageController.java:64 (uploadProductImage — List 파라미터 → 내부에서 배열로 변환)
- src/main/java/com/uhdyl/backend/image/api/ImageApi.java:118, 120
What is this PR?🔍
Changes💻
ImageService의uploadImage호출 시 기존에는MultipartFile로 받은 이미지를uploadImage의 인자로 넘겨주었지만, 현재는MultipartFile타입의 배열을 생성하고, 이 배열을uploadImage의 인자로 넘겨줍니다.uploadImage내에서cloudinary에 저장한 이미지의publicId를 저장하고 업로드 실패 시rollbackImage를 호출하여 이미 업로드된 이미지를cloudinary에서 삭제합니다.rollbackImage가 실행되고 이미지 삭제 실패가 발생하더라도 최대한 많은 이미지를 삭제하도록try catch를 이중으로 사용했습니다.ScreenShot📷
Summary by CodeRabbit
New Features
Bug Fixes