Issue: (#103) 커플 편지 전체보기 수정 #104
Conversation
Walkthrough이번 변경에서는 일정 삭제 시 관련된 편지, 댓글, 사진 및 S3 파일을 모두 삭제하는 계단식 삭제 로직이 도입되었습니다. 또한, 월별 일정 응답에 기념일 정보가 복원되었고, 편지 및 기념일 목록 조회 API의 페이지 인덱스가 1부터 시작하도록 수정되었습니다. 기타 로깅 추가 및 리포지토리 메서드 구현 개선이 포함되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Facade
participant LetterService
participant CommentService
participant PictureService
participant S3Service
participant ScheduleRepository
User->>Controller: 일정 삭제 요청 (scheduleId)
Controller->>Facade: delete(scheduleId)
Facade->>ScheduleRepository: 일정 조회
Facade->>LetterService: 커플+일정ID로 편지 목록 조회
loop 각 편지별
Facade->>PictureService: 편지ID로 사진 목록 조회
loop 각 사진별
Facade->>S3Service: S3에서 파일 삭제
end
Facade->>PictureService: 편지ID로 사진 일괄 삭제
Facade->>CommentService: 편지ID로 댓글 일괄 삭제
Facade->>LetterService: 편지 삭제
end
Facade->>ScheduleRepository: 일정 삭제
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/kotlin/gomushin/backend/core/oauth/handler/CustomAccessDeniedHandler.kt (1)
29-31: 로깅 구현 및 주석에 관한 개선 제안액세스 거부 상황에 대한 로깅을 추가한 것은 보안 문제 디버깅에 좋은 접근입니다. 다만 몇 가지 개선 사항을 제안합니다:
- 29번 줄의 한글 주석은 개인적인 메모로 보입니다. 프로덕션 코드에는 적합하지 않을 수 있습니다.
- 로그 메시지에 추가적인 컨텍스트(예: 요청 URL, 사용자 정보 등)를 포함하면 디버깅이 더 쉬워질 수 있습니다.
- // 정확히 어떤 이유로 발생한 에러인지 보려면 어떻게 해야하나 - logger.error("Access Denied: ${accessDeniedException.message}") + // Log access denied events with context for easier debugging + logger.error("Access Denied for URI [${request.requestURI}]: ${accessDeniedException.message}")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/deploy.yml(1 hunks)src/main/kotlin/gomushin/backend/core/oauth/handler/CustomAccessDeniedHandler.kt(3 hunks)
🔇 Additional comments (3)
.github/workflows/deploy.yml (1)
6-7: GitHub Actions 워크플로우 트리거 조건 확장 적용이 잘 되었습니다.
main브랜치를 대상으로 하는 풀 리퀘스트에 대해서도 워크플로우가 실행되도록 트리거 조건을 확장했습니다. 이는 코드 변경 사항이main브랜치에 병합되기 전에 사전 검증할 수 있게 해주는 좋은 CI/CD 관행입니다.src/main/kotlin/gomushin/backend/core/oauth/handler/CustomAccessDeniedHandler.kt (2)
8-8: 로깅 라이브러리 임포트가 적절히 추가되었습니다.SLF4J Logger를 사용하기 위한 필요한 임포트가 잘 추가되었습니다.
17-17: 로깅 인스턴스 초기화가 잘 구현되었습니다.클래스에 대한 Logger 인스턴스가 적절하게 초기화되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/kotlin/gomushin/backend/schedule/domain/service/LetterService.kt (1)
40-43: 적절한 조회 메소드 추가, 기존 메소드와 중복 가능성 확인 필요새로운
findByCoupleIdAndScheduleId메소드가 추가되어 커플 ID와 일정 ID로 편지를 조회할 수 있게 되었습니다. 이 메소드는 일정 삭제 시 관련 편지를 찾는 데 유용합니다.다만, 67-68줄의
findByCoupleAndSchedule메소드와 기능이 유사해 보입니다. 두 메소드가 각각 다른 컨텍스트에서 사용된다면 문제가 없지만, 코드 중복을 줄이기 위해 통합을 고려해 볼 수 있습니다.src/main/kotlin/gomushin/backend/schedule/facade/UpsertAndDeleteScheduleFacade.kt (1)
35-41: 중복 조회로 인한 성능 저하 가능성
이미findAllByLetter(letter)로 모든Picture엔티티를 가져온 뒤 다시pictureService.deleteAllByLetterId(letter.id)를 호출하면 같은 데이터를 두 번 조회할 수 있습니다.
한 번의 조회 결과를 활용해 S3 URL 목록을 수집하고, 그 결과로 일괄 삭제(쿼리 한 번)를 수행하거나pictureService안에서 S3 삭제와 DB 삭제를 함께 처리하여 조회를 한 번으로 줄여보세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/kotlin/gomushin/backend/schedule/domain/repository/CommentRepository.kt(1 hunks)src/main/kotlin/gomushin/backend/schedule/domain/repository/PictureRepository.kt(1 hunks)src/main/kotlin/gomushin/backend/schedule/domain/service/CommentService.kt(1 hunks)src/main/kotlin/gomushin/backend/schedule/domain/service/LetterService.kt(1 hunks)src/main/kotlin/gomushin/backend/schedule/dto/response/MonthlySchedulesAndAnniversariesResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/schedule/facade/UpsertAndDeleteScheduleFacade.kt(2 hunks)src/main/kotlin/gomushin/backend/schedule/presentation/ReadLetterController.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/kotlin/gomushin/backend/schedule/dto/response/MonthlySchedulesAndAnniversariesResponse.kt
🔇 Additional comments (3)
src/main/kotlin/gomushin/backend/schedule/domain/service/CommentService.kt (1)
70-74: 캐스케이드 삭제 로직 추가 - 좋은 개선점입니다!새로운
deleteAllByLetterId메소드를 추가하여 편지 ID를 기반으로 모든 관련 댓글을 삭제하는 기능이 구현되었습니다. 이는 일정 삭제 시 관련된 모든 데이터를 정리하는 데 중요한 역할을 합니다.src/main/kotlin/gomushin/backend/schedule/domain/repository/CommentRepository.kt (1)
15-18: 벌크 삭제 작업으로 적절히 구현되었습니다
@Modifying어노테이션과 JPQL 쿼리를 사용하여 특정 편지 ID와 관련된 모든 댓글을 효율적으로 삭제하는 메소드가 추가되었습니다. 이 구현은 개별 엔티티를
로드하고 삭제하는 것보다 성능이 더 좋을 것입니다.src/main/kotlin/gomushin/backend/schedule/domain/repository/PictureRepository.kt (1)
14-16: 벌크 삭제로 성능 개선된 구현기존의 파생 쿼리 메소드에서
@Modifying과 JPQL을 사용한 커스텀 쿼리로 변경되었습니다. 이 변경은 CommentRepository의 구현과 일관성을 유지하며, 대량의 Picture 엔티티를 삭제할 때 성능을 향상시킬 것입니다.이는 일정 삭제 시 관련된 모든 리소스를 효율적으로 정리하는 캐스케이드 삭제 전략의 일부로 적절하게 구현되었습니다.
src/main/kotlin/gomushin/backend/schedule/presentation/ReadLetterController.kt
Outdated
Show resolved
Hide resolved
| @Transactional | ||
| fun delete(customUserDetails: CustomUserDetails, scheduleId: Long) { | ||
| val schedule = scheduleService.getById(scheduleId) | ||
| letterService.findByCoupleAndSchedule(customUserDetails.getCouple(), schedule).forEach { letter -> | ||
| pictureService.findAllByLetter(letter) | ||
| .takeIf { it.isNotEmpty() } | ||
| ?.forEach { picture -> | ||
| s3Service.deleteFile(picture.pictureUrl) | ||
| } | ||
| pictureService.deleteAllByLetterId(letter.id) | ||
| commentService.deleteAllByLetterId(letter.id) | ||
| letterService.delete(letter.id) | ||
| } | ||
| scheduleService.delete(customUserDetails.getCouple().id, customUserDetails.getId(), scheduleId) |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
트랜잭션 내부에서 S3 삭제 수행 시 원자성 보장 불가
@Transactional 블록 안에서 S3 객체를 즉시 삭제하면 DB 롤백 시점과 S3 상태가 불일치할 위험이 있습니다.
- S3 삭제가 실패하면 전체 트랜잭션이 롤백되더라도 이미 일부 객체가 삭제되지 않은 채로 DB는 되돌아갈 수 있고,
- 반대로 DB 롤백 시 S3 는 이미 삭제가 완료되어 복구가 어렵습니다.
권장 방안
- S3 삭제를 트랜잭션 외부(커밋 이후)로 이동하거나
- 아웃박스/이벤트 발행 패턴으로 S3 삭제를 비동기 수행해 DB-S3 간 일관성을 유지하세요.
트랜잭션 커밋 이후로 S3 삭제 로직 분리하기
트랜잭션 경계(@Transactional) 안에서 s3Service.deleteFile(...)을 호출하면 DB 롤백 시점과 S3 상태가 불일치할 수 있습니다.
- 위치
- 파일:
src/main/kotlin/gomushin/backend/schedule/facade/UpsertAndDeleteScheduleFacade.kt - 메서드:
delete(31–44번 라인 내부)
- 파일:
- 문제 요약
- DB 트랜잭션이 롤백되어도 이미 삭제된 일부 S3 객체는 복구 불가
- S3 삭제 실패 시 트랜잭션은 롤백되지만, 일부 객체가 남아 일관성 위배
- 권장 조치
- S3 삭제 로직을 트랜잭션 커밋 이후(또는 트랜잭션 바깥)로 이동
- 아웃박스/이벤트 발행 패턴을 도입해 비동기 방식으로 삭제 처리
| @RequestParam(defaultValue = "10") size: Int, | ||
| ): PageResponse<TotalAnniversaryResponse> { | ||
| val anniversaries = anniversaryFacade.getAnniversaryList(customUserDetails, page, size) | ||
| val safePage = if (page < 1) 0 else page - 1 |
| @RequestParam(defaultValue = "10") size: Int, | ||
| ): PageResponse<LetterPreviewResponse> { | ||
| val letters = readLetterFacade.getLetterListToCouple(customUserDetails, page, size); | ||
| val safePage = if (page < 1) 0 else page - 1 |
There was a problem hiding this comment.
프론트가 page값을 0보다 작게주는 경우가 있나?
There was a problem hiding this comment.
프론트에서 페이지를 1부터 보내서, 기준을 1로 잡았어
그래서 만약 억지로 0을 보내게 되면, 서버에선 -1로 처리하니까 에러가 발생해
여기서 발생할 수 있는 에러를 막으려고 0 이하는 전부 0으로 처리하도록 한거야
✅ PR 유형
어떤 변경 사항이 있었나요?
📝 작업 내용
✏️ 관련 이슈
🎸 기타 사항 or 추가 코멘트
Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
버그 수정
개선 사항