Merged
Conversation
Walkthrough사용자 프로필 이미지 업데이트 기능을 구현합니다. 도메인 모델에 Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as UserImageController
participant Service as UserImageService
participant Validation as ValidationService
participant Repository as UserImageRepository
participant Util as ImageUtil
participant Domain as Image Domain
User->>Controller: PATCH /user/image<br/>(imagePath, imagePrefix)
Controller->>Service: updateImage(userId, imagePrefix, filename, path)
rect rgb(240, 248, 255)
Note over Service: 검증 단계
Service->>Repository: findByUserId(userId)
Service->>Validation: validateDuplicatedImagePath(path)
Service->>Validation: validateUserImageOwnership(userId, image)
Service->>Service: parseImageName(filename)
end
rect rgb(240, 255, 240)
Note over Service: 업데이트 단계
Service->>Domain: updateImage(path, filename, name, extension, prefix)
Service->>Repository: save(userImageEntity)
end
rect rgb(255, 250, 240)
Note over Service: 정리 단계
Service->>Util: deleteFile(oldImagePath)
end
Service->>Controller: return UserImageEntity
Controller->>User: 200 OK<br/>(CallbackImageSaveUrlResponse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (5)
src/main/java/hanium/modic/backend/domain/image/domain/Image.java (1)
40-52: null 값 조기 검증으로 무결성 보장updateImage가 @column(nullable = false) 필드들에 그대로 대입합니다. null이 유입되면 커밋 시점에야 폭발하므로 메서드 진입 시 즉시 검증하는 편이 안전합니다.
적용 예시:
public void updateImage( String imagePath, String fullImageName, String imageName, ImageExtension extension, ImagePrefix imagePurpose ) { - this.imagePath = imagePath; - this.fullImageName = fullImageName; - this.imageName = imageName; - this.extension = extension; - this.imagePurpose = imagePurpose; + this.imagePath = Objects.requireNonNull(imagePath, "imagePath"); + this.fullImageName = Objects.requireNonNull(fullImageName, "fullImageName"); + this.imageName = Objects.requireNonNull(imageName, "imageName"); + this.extension = Objects.requireNonNull(extension, "extension"); + this.imagePurpose = Objects.requireNonNull(imagePurpose, "imagePurpose"); }추가 import:
import java.util.Objects;src/main/java/hanium/modic/backend/domain/user/service/UserImageService.java (2)
119-123: 경로 중복 검증이 자기 자신까지 포함됨(업데이트 불가 케이스) + 소유권 검증 중복 가능성
- 기존 경로를 그대로 두는 업데이트에서도 existsByImagePath(imagePath)가 true가 되어 불필요하게 실패할 수 있습니다. 자기 자신은 제외하거나, 경로가 불변이면 스킵하세요.
- findByUserId로 이미 본인 레코드를 조회했으므로 추가 소유권 검증은 중복일 수 있습니다(정책에 따라 유지 가능).
최소 변경 예:
- imageValidationService.validateImageSaved(imagePath); - validateDuplicatedImagePath(imagePath); - validateUserImageOwnership(userId, userImage.getId()); + imageValidationService.validateImageSaved(imagePath); + if (!imagePath.equals(oldImagePath)) { + validateDuplicatedImagePath(imagePath); + } + // 필요 시 정책에 따라 유지: validateUserImageOwnership(userId, userImage.getId());검증 강화(선택): 저장소에
existsByImagePathAndIdNot(String path, Long id)를 추가해 현재 레코드 제외 검증을 권장합니다.
134-135: save 호출 불필요 — 더티 체킹으로 반영됨userImage는 영속 상태이므로 save 재호출 없이 변경사항이 플러시/커밋됩니다. 불필요한 DB I/O 제거를 권장합니다.
- userImage = userImageRepository.save(userImage);src/main/java/hanium/modic/backend/web/user/controller/UserImageController.java (2)
111-117: PATCH 엔드포인트의 오류 매핑에 IMAGE_NOT_FOUND_EXCEPTION 누락updateImage는 기존 이미지를 찾지 못하면 IMAGE_NOT_FOUND_EXCEPTION을 던집니다. 문서/핸들링 일관성을 위해 매핑에 포함하는 것을 권장합니다.
@ApiErrorMapping({ USER_INPUT_EXCEPTION, IMAGE_NOT_STORE_EXCEPTION, INVALID_IMAGE_FILE_NAME_EXCEPTION, IMAGE_PATH_DUPLICATED_EXCEPTION, - USER_ROLE_EXCEPTION + USER_ROLE_EXCEPTION, + IMAGE_NOT_FOUND_EXCEPTION })
118-130: 요청 DTO 명/의도 명확화 제안PATCH에서도 CallbackImageSaveUrlRequest를 재사용하면 “저장 콜백”과 “변경”의 의미가 모호해질 수 있습니다. UpdateUserImageRequest 등 의미가 드러나는 DTO로 분리하면 API 사용성이 좋아집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/hanium/modic/backend/domain/image/domain/Image.java(1 hunks)src/main/java/hanium/modic/backend/domain/user/service/UserImageService.java(1 hunks)src/main/java/hanium/modic/backend/web/user/controller/UserImageController.java(5 hunks)
Comment on lines
+136
to
+139
| // 기존 이미지 삭제 | ||
| imageUtil.deleteImage(userImage.getImagePath()); | ||
|
|
||
| return userImage; |
There was a problem hiding this comment.
새 이미지가 삭제되는 치명적 버그 + 커밋 이후 삭제 필요
userImage.updateImage 이후 userImage.getImagePath()는 “새 경로”를 가리킵니다. 현재 코드는 새 이미지를 지우게 됩니다. 또한 파일 삭제는 트랜잭션 커밋 후 실행해야 롤백 시 데이터 손실을 막을 수 있습니다.
권장 수정:
- // 기존 이미지 삭제
- imageUtil.deleteImage(userImage.getImagePath());
+ // 기존 이미지 삭제는 트랜잭션 커밋 후 실행
+ final String oldImagePathFinal = oldImagePath; // 아래 diff에서 oldImagePath를 추가합니다.
+ TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
+ @Override
+ public void afterCommit() {
+ imageUtil.deleteImage(oldImagePathFinal);
+ }
+ });위 수정이 동작하려면, 기존 이미지 경로를 업데이트 전에 보관하세요:
- UserImageEntity userImage = userImageRepository.findByUserId(userId)
- .orElseThrow(() -> new AppException(IMAGE_NOT_FOUND_EXCEPTION));
+ UserImageEntity userImage = userImageRepository.findByUserId(userId)
+ .orElseThrow(() -> new AppException(IMAGE_NOT_FOUND_EXCEPTION));
+ final String oldImagePath = userImage.getImagePath();필요 import:
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;🤖 Prompt for AI Agents
In src/main/java/hanium/modic/backend/domain/user/service/UserImageService.java
around lines 136 to 139, the code deletes userImage.getImagePath() after calling
userImage.updateImage(...) which now points to the new image — causing the new
image to be removed — and the file deletion is executed inside the transaction
(risking data loss on rollback). Fix by capturing the existing image path into a
local variable before calling updateImage(), then register a
TransactionSynchronization via
TransactionSynchronizationManager.registerSynchronization so that
imageUtil.deleteImage(oldPath) runs in afterCommit(); also add null/empty checks
and skip deletion if oldPath equals the new path. Ensure you import
org.springframework.transaction.support.TransactionSynchronization and
org.springframework.transaction.support.TransactionSynchronizationManager.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
작업사항
Summary by CodeRabbit
릴리스 노트
새 기능
개선