-
Notifications
You must be signed in to change notification settings - Fork 0
[BUG] 사용자 기본정보 수정 API 누락 #181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthrough인증된 사용자의 기본 정보(닉네임·프로필 사진·공개여부)를 PATCH /me로 갱신하는 API를 추가하고, 프로필 변경 시 기존 이미지 삭제를 트랜잭션 커밋 후 이벤트 기반으로 S3에서 처리하도록 구현했습니다. (약 50자) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as 클라이언트
participant Controller as UserController
participant Service as UserServiceImpl
participant DB as Database/Transaction
participant EventPub as ApplicationEventPublisher
participant Listener as ProfileImageEventListener
participant S3 as S3Service
Client->>Controller: PATCH /me (UpdateUserInfoRequestDto)
Controller->>Service: updateUserInfo(userId, dto)
Service->>DB: 사용자 정보 갱신 (새 이미지 키 저장)
DB-->>Service: 트랜잭션(커밋 준비)
alt 이전 이미지 키 존재
Service->>EventPub: publish(ProfileImageDeletedEvent(oldImageKey))
end
Service-->>Controller: UpdateUserInfoResponseDto + USER_UPDATE_OK
DB->>DB: 트랜잭션 커밋
Note right of DB: AFTER_COMMIT 트리거
EventPub->>Listener: 전달(ProfileImageDeletedEvent)
Listener->>S3: deleteFileByKey(oldImageKey)
S3-->>Listener: 삭제 완료
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/hrr/backend/domain/user/controller/UserController.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:06:38.145Z
Learnt from: yerinchun
Repo: Hrr-HabbitRoutineResult/hrr-server PR: 26
File: src/main/java/com/hrr/backend/domain/user/entity/User.java:42-44
Timestamp: 2025-11-06T08:06:38.145Z
Learning: In the hrr-server project User entity (src/main/java/com/hrr/backend/domain/user/entity/User.java), the password field is nullable because the application uses social login (OAuth), and social login users do not have passwords stored in the database.
Applied to files:
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🔇 Additional comments (2)
src/main/java/com/hrr/backend/domain/user/controller/UserController.java (1)
165-177: 잘 복구하셨네요! 🎯누락된 API가 깔끔하게 복원되었습니다. 인증, 검증, 응답 구조 모두 기존 패턴과 일관성 있게 구현되었어요.
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
18-18: 의존성 주입이 올바르게 구성되었습니다.S3Service가 적절히 추가되었고, Lombok의 @requiredargsconstructor를 통해 생성자 주입이 자동으로 처리됩니다.
Also applies to: 56-56
| // 기존 이미지가 있으면 S3에서 삭제 | ||
| String oldImageKey = user.getProfileImage(); | ||
| if (oldImageKey != null && !oldImageKey.isBlank()) { | ||
| s3Service.deleteFileByKey(oldImageKey); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
S3 삭제 로직에 개선이 필요합니다.
현재 구현에는 몇 가지 잠재적 문제가 있습니다:
- 동일 키 체크 누락: 사용자가 동일한 이미지를 다시 설정하면 해당 이미지가 삭제된 후 다시 설정됩니다.
- 트랜잭션 경계 문제: S3 삭제가 실패하면 전체 트랜잭션이 롤백되어 프로필 업데이트 자체가 불가능해집니다. 외부 시스템(S3) 호출을 트랜잭션 내부에서 수행하면 이런 문제가 발생할 수 있습니다.
🔎 개선 방안
방안 1 (권장): 동일 키 체크 추가 + 삭제 실패 시 로깅만 수행
// 프로필 이미지 Key가 제공된 경우 업데이트
if (requestDto.getProfileImageKey() != null) {
+ String newImageKey = requestDto.getProfileImageKey();
// 기존 이미지가 있으면 S3에서 삭제
String oldImageKey = user.getProfileImage();
- if (oldImageKey != null && !oldImageKey.isBlank()) {
+ if (oldImageKey != null && !oldImageKey.isBlank() && !oldImageKey.equals(newImageKey)) {
- s3Service.deleteFileByKey(oldImageKey);
+ try {
+ s3Service.deleteFileByKey(oldImageKey);
+ } catch (Exception e) {
+ // S3 삭제 실패 시 로깅만 하고 계속 진행
+ log.warn("Failed to delete old profile image: {}", oldImageKey, e);
+ }
}
- user.updateProfileImage(requestDto.getProfileImageKey());
+ user.updateProfileImage(newImageKey);
}방안 2 (대안): 트랜잭션 커밋 후 삭제 (비동기 또는 @TransactionalEventListener 활용)
S3 삭제를 DB 커밋 이후로 지연시켜 트랜잭션 독립성을 보장하는 방법입니다. 이 경우 일시적으로 불필요한 파일이 S3에 남을 수 있지만, 사용자 경험에는 영향을 주지 않습니다.
참고 자료:
- Spring Transaction Management Best Practices
- 외부 시스템 호출과 트랜잭션 경계에 대한 고려사항
🤖 Prompt for AI Agents
In src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java around
lines 309 to 313, the S3 delete logic should avoid deleting when the new image
key is identical and must not perform S3 deletion inside the DB transaction;
update the code to first compare the existing profileImage key with the
incoming/new key and skip deletion if they are equal or blank, and if deletion
is needed either (a) attempt deletion but catch exceptions and only log failures
(do not let S3 errors propagate and rollback the transaction) or (b) move the
deletion to execute after transaction commit (e.g., publish an event or run
asynchronously via @TransactionalEventListener or a separate background task) so
S3 removals occur outside the transactional boundary.
yc3697
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드래빗 리뷰 달린 내용만 수정해주시면 될 것 같아요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
309-319: [중요] 동일한 이미지 키 체크가 누락되었습니다.현재 코드는 사용자가 동일한 이미지로 "업데이트"할 경우, 해당 이미지를 S3에서 삭제해버리는 문제가 있습니다.
발생 시나리오:
- 사용자가 현재 프로필 이미지와 동일한 이미지 키를 다시 제출
- Line 310에서
oldImageKey는 현재 이미지 키를 저장- Line 313에서 동일한 키로 업데이트 (실질적 변경 없음)
- Line 316-318에서
oldImageKey가 null이 아니므로 삭제 이벤트 발행- 트랜잭션 커밋 후 리스너가 현재 사용 중인 이미지를 S3에서 삭제
- 결과: DB에는 올바른 키가 있지만 S3에 파일이 없어 이미지 깨짐
이 문제는 이전 리뷰에서도 지적되었으나 해결되지 않았습니다.
🔎 수정 방안
// 프로필 이미지 Key가 제공된 경우 업데이트 if (requestDto.getProfileImageKey() != null) { + String newImageKey = requestDto.getProfileImageKey(); String oldImageKey = user.getProfileImage(); // 새 이미지로 업데이트 - user.updateProfileImage(requestDto.getProfileImageKey()); + user.updateProfileImage(newImageKey); // 트랜잭션 커밋 후 기존 이미지 삭제 이벤트 발행 - if (oldImageKey != null && !oldImageKey.isBlank()) { + // 동일한 이미지인 경우 삭제하지 않음 + if (oldImageKey != null && !oldImageKey.isBlank() && !oldImageKey.equals(newImageKey)) { eventPublisher.publishEvent(new ProfileImageDeletedEvent(oldImageKey)); } }참고: 이벤트 기반 접근으로 트랜잭션 경계 문제는 잘 해결되었지만, 동일 키 검증은 여전히 필요합니다.
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/domain/user/event/ProfileImageEventListener.java (1)
27-36: 트랜잭션 커밋 후 처리 로직이 올바르게 구현되었습니다.
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)를 사용하여 이전 리뷰에서 지적된 트랜잭션 경계 문제를 제대로 해결했습니다.다만, S3 삭제 실패 시 예외 처리가 없어 운영상 리스크가 있을 수 있습니다. DB는 이미 커밋된 상태이므로 사용자에게는 문제가 없지만, S3에 불필요한 파일이 남거나 모니터링이 어려울 수 있습니다.
🔎 예외 처리 추가 제안
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void handleProfileImageDeleted(ProfileImageDeletedEvent event) { String oldImageKey = event.getOldImageKey(); if (oldImageKey != null && !oldImageKey.isBlank()) { log.info("프로필 이미지 삭제 시작 (트랜잭션 커밋 후): {}", oldImageKey); - s3Service.deleteFileByKey(oldImageKey); - log.info("프로필 이미지 삭제 완료: {}", oldImageKey); + try { + s3Service.deleteFileByKey(oldImageKey); + log.info("프로필 이미지 삭제 완료: {}", oldImageKey); + } catch (Exception e) { + log.error("프로필 이미지 삭제 실패 (S3 오류): {}", oldImageKey, e); + // 필요시 재시도 큐에 추가하거나 알림 발송 + } } }참고: 현재 구조에서는 S3 실패가 사용자 경험에 영향을 주지 않지만, 장애 추적 및 재처리를 위해 예외 로깅을 권장합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/hrr/backend/domain/user/event/ProfileImageDeletedEvent.javasrc/main/java/com/hrr/backend/domain/user/event/ProfileImageEventListener.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🔇 Additional comments (1)
src/main/java/com/hrr/backend/domain/user/event/ProfileImageDeletedEvent.java (1)
1-14: 이벤트 클래스 설계가 깔끔합니다! 👍불변 이벤트 객체로 설계되어 있고, 트랜잭션 커밋 후 S3 삭제를 위한 명확한 목적을 가지고 있습니다. Lombok 애노테이션도 적절하게 사용되었습니다.
yc3697
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다 수고하셨어요~
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
누락된 코드 다시 추가
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.