[WTH-415] 파일 삭제 구현#83
Open
soo0711 wants to merge 15 commits into
Hidden character warning
The head ref may contain hidden characters: "feat/WTH-415-\ud30c\uc77c-\uc0ad\uc81c-\uad6c\ud604"
Open
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
📝 WalkthroughWalkthrough파일 엔티티와 저장소가 삭제 예약 메타데이터를 지원하도록 확장됐고, 수취·게시글·댓글·클럽·탈퇴 관련 유스케이스와 정책이 즉시 삭제 대신 활성 삭제 마킹과 하드 삭제 시점 기록을 사용하도록 바뀌었다. 관련 테스트와 조회 조건도 함께 갱신됐다. Changes파일 삭제 예약 전환
Sequence Diagram(s)sequenceDiagram
participant ClubActivityDeletionPolicy
participant CommentRepository
participant FileRepository
participant PostLikeRepository
participant PostRepository
ClubActivityDeletionPolicy->>CommentRepository: findActiveIdsByClubMemberIdIn(memberIds)
ClubActivityDeletionPolicy->>FileRepository: markActiveDeletedByOwnerTypeAndOwnerIdIn(...)
ClubActivityDeletionPolicy->>PostLikeRepository: findActivePostIdsByUserIdAndClubIdIn(userId, clubIds)
ClubActivityDeletionPolicy->>PostRepository: findAllByIdsWithLock(postIds)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt (1)
416-422: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
hardDeleteAfter기대값에 삭제 정책 helper를 사용해 주세요.여기도 production 경로는
File.immediateHardDeleteAfter(now)를 쓰므로, 테스트가 rawLocalDateTime.now(clock)에 결합되지 않게 같은 helper로 기대값을 만들면 정책 변경 시 테스트가 덜 깨집니다.🧪 제안 수정 패턴
+ val deletedAt = LocalDateTime.now(clock) verify(exactly = 1) { fileRepository.markActiveDeletedByOwnerTypeAndOwnerId( FileOwnerType.CLUB_PROFILE, 1L, - LocalDateTime.now(clock), - LocalDateTime.now(clock), + deletedAt, + File.immediateHardDeleteAfter(deletedAt), ) }
deleteProfileImage,deleteBackgroundImage검증도 같은 방식으로 맞추면 됩니다.Also applies to: 521-527, 580-586
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt` around lines 416 - 422, The test expectations in ManageClubUseCaseTest are coupled to raw LocalDateTime.now(clock) values instead of the same delete-policy helper used in production. Update the verify() assertions for deleteProfileImage, deleteBackgroundImage, and the related hard delete checks to build the expected hardDeleteAfter value with File.immediateHardDeleteAfter(now) (using the same now source as the test), so the assertions match the production path and stay aligned if the policy changes.src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.kt (1)
240-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win테스트 기대값도 삭제 정책 헬퍼를 따라가게 해주세요.
Production 코드는
File.immediateHardDeleteAfter(now)를 전달하는데, 테스트는hardDeleteAfter를LocalDateTime.now(clock)로 고정하고 있습니다. 정책 helper가 바뀌면 테스트가 구현 정책과 어긋나므로 동일 helper를 사용해 검증하는 편이 안전합니다.🧪 제안 수정
+ val deletedAt = LocalDateTime.now(clock) verify(exactly = 1) { fileRepository.markActiveDeletedByOwnerTypeAndOwnerId( FileOwnerType.POST, post.id, - LocalDateTime.now(clock), - LocalDateTime.now(clock), + deletedAt, + File.immediateHardDeleteAfter(deletedAt), ) }+ val deletedAt = LocalDateTime.now(clock) verify(exactly = 1) { fileRepository.markActiveDeletedByOwnerTypeAndOwnerId( FileOwnerType.POST, post.id, - LocalDateTime.now(clock), - LocalDateTime.now(clock), + deletedAt, + File.immediateHardDeleteAfter(deletedAt), ) }Also applies to: 353-359
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.kt` around lines 240 - 246, The test expectation in ManagePostUseCaseTest is hardcoding the delete policy timestamp with LocalDateTime.now(clock) instead of following the same helper used by production code. Update the verifications around markActiveDeletedByOwnerTypeAndOwnerId to use the same deletion-policy helper as the implementation, specifically File.immediateHardDeleteAfter(now), so the test stays aligned if the policy changes. Apply this fix to both affected verification blocks in the test class.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/kotlin/com/weeth/domain/club/domain/service/ClubActivityDeletionPolicy.kt`:
- Around line 81-98: The like cleanup in ClubActivityDeletionPolicy still drops
deleted Post/Board IDs because it filters through findAllByIdsWithLock() before
markDeleted(now), so inactive posts never get their remaining active PostLike
rows removed. Refactor the flow around findActivePostIdsByUserIdAndClubIdIn(),
findAllByIdsWithLock(), and findAllActiveByUserIdAndPostIds() so like deletion
is based on the originally fetched postIds (including soft-deleted ones), while
only the like count decrement is limited to posts that are still active. Keep
the lock/query for active posts separate from the deletion of PostLike rows so
deleted content does not prevent cleanup.
In `@src/main/kotlin/com/weeth/domain/file/domain/entity/File.kt`:
- Around line 52-61: The File entity now includes isDeleted, deletedAt, and
hardDeleteAfter, but the corresponding database schema change is missing. Add a
Flyway migration under the existing db migration flow that alters the file table
to include these columns so File and its persistence mapping stay aligned with
ddl-auto validate and production startup in application-prod.yml. Use the File
entity field names and the existing migration versioning pattern (for example
the V2/V4 scripts) to locate where the new ALTER TABLE file changes should be
added.
In `@src/main/kotlin/com/weeth/domain/file/domain/repository/FileRepository.kt`:
- Around line 49-68: `FileRepository`의 bulk update 메서드인
`markActiveDeletedByOwnerTypeAndOwnerId`가 `@Modifying(flushAutomatically =
true)`만 사용해 영속성 컨텍스트가 남을 수 있습니다. `@Modifying`에 `clearAutomatically = true`를 추가해
업데이트 후 `File` 엔티티가 같은 트랜잭션에서 stale 상태로 보이지 않도록 수정하고, 이 repository 안의 다른 bulk
update 메서드들도 동일한 방식으로 적용해 일관되게 동기화하세요.
In
`@src/test/kotlin/com/weeth/domain/file/domain/repository/FileRepositoryTest.kt`:
- Around line 275-280: The Map<String, Any?>.booleanBy helper is too permissive
because value.toString().toBoolean() silently turns null, unexpected strings
like "Y", and typos into false. Tighten the JDBC test helper by only accepting
supported boolean representations in booleanBy and failing fast for anything
else, using the existing booleanBy extension in FileRepositoryTest to locate the
change.
---
Nitpick comments:
In
`@src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.kt`:
- Around line 240-246: The test expectation in ManagePostUseCaseTest is
hardcoding the delete policy timestamp with LocalDateTime.now(clock) instead of
following the same helper used by production code. Update the verifications
around markActiveDeletedByOwnerTypeAndOwnerId to use the same deletion-policy
helper as the implementation, specifically File.immediateHardDeleteAfter(now),
so the test stays aligned if the policy changes. Apply this fix to both affected
verification blocks in the test class.
In
`@src/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt`:
- Around line 416-422: The test expectations in ManageClubUseCaseTest are
coupled to raw LocalDateTime.now(clock) values instead of the same delete-policy
helper used in production. Update the verify() assertions for
deleteProfileImage, deleteBackgroundImage, and the related hard delete checks to
build the expected hardDeleteAfter value with File.immediateHardDeleteAfter(now)
(using the same now source as the test), so the assertions match the production
path and stay aligned if the policy changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81fb317d-1b32-41a4-a1d2-c6e03323f98a
📒 Files selected for processing (23)
src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCase.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/PostLikeRepository.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCase.ktsrc/main/kotlin/com/weeth/domain/club/domain/service/ClubActivityDeletionPolicy.ktsrc/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.ktsrc/main/kotlin/com/weeth/domain/comment/domain/repository/CommentRepository.ktsrc/main/kotlin/com/weeth/domain/file/domain/entity/File.ktsrc/main/kotlin/com/weeth/domain/file/domain/repository/FileRepository.ktsrc/main/kotlin/com/weeth/domain/user/application/usecase/command/LeaveUserUseCase.ktsrc/test/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/club/domain/service/ClubActivityDeletionPolicyTest.ktsrc/test/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/comment/application/usecase/query/GetCommentQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/file/domain/entity/FileTest.ktsrc/test/kotlin/com/weeth/domain/file/domain/repository/FileRepositoryTest.ktsrc/test/kotlin/com/weeth/domain/user/application/usecase/command/LeaveUserUseCaseTest.kt
Comment on lines
+52
to
+61
| @Column(nullable = false) | ||
| var isDeleted: Boolean = isDeleted | ||
| private set | ||
|
|
||
| @Column(name = "deleted_at", nullable = true) | ||
| var deletedAt: LocalDateTime? = deletedAt | ||
| private set | ||
|
|
||
| @Column(name = "hard_delete_after", nullable = true) | ||
| var hardDeleteAfter: LocalDateTime? = hardDeleteAfter |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 삭제 예약 컬럼 DDL이 현재 PR/브랜치에 포함되어 있는지 확인합니다.
# Expect: 배포 가능한 단위라면 file 테이블에 is_deleted, deleted_at, hard_delete_after를 추가/backfill하는 migration이 있어야 합니다.
rg -n "is_deleted|deleted_at|hard_delete_after|ALTER[[:space:]]+TABLE.*file|CREATE[[:space:]]+TABLE.*file" .
rg -n "flyway|liquibase|ddl-auto|hibernate\\.hbm2ddl" -C 2 .Repository: Team-Weeth/weeth-server
Length of output: 8937
🏁 Script executed:
#!/bin/bash
# Migration 파일 목록 확인
ls -lt src/main/resources/db/migration/Repository: Team-Weeth/weeth-server
Length of output: 528
시각적 스키마 마이그레이션 스크립트가 누락되었습니다
File 엔티티에 isDeleted, deletedAt, hardDeleteAfter 필드가 추가되었으나, 이를 DB 에 반영하는 Flyway 마이그레이션 스크립트 ( ALTER TABLE file ...) 가 누락되었습니다.
프로덕션 환경 (application-prod.yml) 에서 Flyway 가 활성화되고 ddl-auto: validate로 설정되어 있어, 스키마 변경이 없으면 애플리케이션 시작 시점에 실패하거나, 컬럼 없어서 SQL 실패가 발생합니다.
- 현재
V2,V4마이그레이션 파일은club_member,users등 다른 테이블만 포함하고file테이블 수정 내용은 없습니다. docs/plan에 초안은 있으나 실제src/main/resources/db/migration/에 추가되어야 합니다.
마이그레이션 스크립트를 추가하거나, 해당 필드의 DB 적용 단계를 명확히 하고 배포 순서를 엄격히 준수해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/kotlin/com/weeth/domain/file/domain/entity/File.kt` around lines 52
- 61, The File entity now includes isDeleted, deletedAt, and hardDeleteAfter,
but the corresponding database schema change is missing. Add a Flyway migration
under the existing db migration flow that alters the file table to include these
columns so File and its persistence mapping stay aligned with ddl-auto validate
and production startup in application-prod.yml. Use the File entity field names
and the existing migration versioning pattern (for example the V2/V4 scripts) to
locate where the new ALTER TABLE file changes should be added.
Contributor
There was a problem hiding this comment.
해당 PR에서 마이그레이션이 진행 되어야 빌드가 정상 동작하지 않을까용??
There was a problem hiding this comment.
@hyxklee 맞습니다. 제가 걱정한 포인트도 정확히 그 부분이에요.
엄밀히 말하면 CI의 “빌드” 자체는 통과할 수도 있지만, 이 변경이 배포 가능한 단위인가 관점에서는 마이그레이션이 같이 가는 편이 가장 안전합니다.
지금처럼 File 엔티티에 새 컬럼 매핑이 추가된 상태에서 애플리케이션이 먼저 배포되면, 운영 환경에서는 validate나 실제 쿼리 시점에 컬럼 부재로 실패할 수 있습니다.
그래서 제 코멘트의 의도는:
- 이 PR에 마이그레이션을 같이 포함하거나
- 정말 후속 PR로 분리할 거라면 “DB 마이그레이션 선반영 → 애플리케이션 배포” 순서가 강제되는지를 명확히 해야 한다
는 의미였습니다.
즉, 질문 주신 표현대로 보면 “정상 동작 보장”을 위해서는 이번 변경과 같은 배포 단위에서 마이그레이션이 필요하다 쪽에 가깝습니다.
🧠 Learnings used
Learnt from: hyxklee
Repo: Team-Weeth/weeth-server PR: 44
File: src/main/kotlin/com/weeth/domain/user/presentation/SocialCallbackController.kt:55-60
Timestamp: 2026-03-31T06:54:23.087Z
Learning: When reviewing Spring code in Kotlin, note that `UriComponentsBuilder.toUriString()` performs URL encoding internally (it effectively calls `encode().build().toUriString()`), including non-ASCII characters such as Korean. Therefore, do not flag `UriComponentsBuilder.toUriString()` as “missing encoding”. Only `UriComponents.toUriString()` is simple string concatenation and does not apply encoding by default.
Collaborator
Author
There was a problem hiding this comment.
해당 PR에서 마이그레이션이 진행 되어야 빌드가 정상 동작하지 않을까용??
아 맞습니다!! 제가 잘못하고 저렇게 적어버렷네용.. 수정해두겟습니당
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
hyxklee
reviewed
Jun 28, 2026
| fileRepository.flush() | ||
| } | ||
| val now = LocalDateTime.now(clock) | ||
| fileRepository.markActiveDeletedByOwnerTypeAndOwnerId( |
Contributor
There was a problem hiding this comment.
해당 메서드는 실제로 DB에서 삭제를 하진 않는 것으로 이해가 되는데, 위 API를 실제로 호출하게 되면 file의 삭제가 이루어지지 않아서 유니크 제약조건에서 fail이 발생할 수도 있을 것 같아요!
이전에 해당 문제가 발생했어서 명시적으로 flush를 해줬던건데 (delete가 insert 보다 늦게 동작하기 때문) 이 로직에서는 해당 문제는 없을까용??
Collaborator
Author
There was a problem hiding this comment.
기존에는 delete가 insert보다 늦게 실행되면서 문제가 발생했는데 현재 로직에서는 삭제 마킹 update가 먼저 DB에 반영되도록 되어 있습니다!
말씀 주신 것처럼 소프트 딜리트라서 row는 남기 때문에 동일한 storageKey로 다시 insert되면 unique 제약에 걸릴 수는 있을 것 같습니당.. 하지만 현재 이미지 업로드 플로우에서는 UUID 기반으로 매번 새로운 storageKey를 생성하고 있어서 정상 플로우에서는 동일 storageKey가 재사용되지 않아 괜찮다고 판단했습니다!
hyxklee
reviewed
Jun 28, 2026
| FileStatus.UPLOADED, | ||
| ) | ||
| if (existingFiles.isNotEmpty()) { | ||
| fileRepository.deleteAll(existingFiles) |
Contributor
There was a problem hiding this comment.
근데 기존에 하드 딜리트를 하던 파트를 소프트 딜리트 + 즉시 정리 대상으로 표시하는 방향으로 수정된 이유가 있을까요??
어차피 즉시 정리 대상으로 처리할 것이라면 하드 딜리트를 해도 문제가 없지 않나 싶어서요!
Collaborator
Author
There was a problem hiding this comment.
이 부분은 DB와 S3 삭제 시점이 분리되어 있어서 소프트 딜리트 + 즉시 정리 대상으로 바꿨습니다!
하드 딜리트를 바로 해버리면 DB row는 삭제됐는데 S3 삭제가 실패하거나 반대로 S3를 먼저 삭제했는데 이후 트랜잭션이 롤백되면서 DB에는 파일 정보가 남아 있는데 S3는 없는 상태가 생길 수 있다고 생각했습니닷
그래서 API 호출 시점에는 hardDeleteAfter = now로 즉시 정리 대상임을 표시한 뒤 클린업 과정에서 S3 삭제 성공 후 하드 딜리트를 처리하는 것이 데이터 정합성 측면에서 더 적절하다고 생각해서 해당 방식으로 수정했습니다!!
이 방향에 대해서는 어떻게 생각하시나용??
hyxklee
reviewed
Jun 28, 2026
hyxklee
left a comment
hyxklee
left a comment
Contributor
There was a problem hiding this comment.
고생하셨습니다! 궁금한 점이 있어서 리뷰 몇 개 달아뒀어용
그럼 프로필 작업 이후에 스케줄링을 통한 하드 딜리트 작업이 예정되어 있는 거졍??
Collaborator
Author
|
넵! 프로필 작업 이후에 스케줄링을 통한 하드 딜리트 작업예정입니다!! |
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
2 participants
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
파일 삭제를 예약하고 탈퇴/삭제 시 연관 파일 정리하도록 구현했습니다.
📝 Changes
What
File 엔티티에 삭제 예약 메타데이터를 추가했습니다.
게시글/댓글/영수증 파일 삭제 및 교체 시 기존 파일을 즉시 정리 대상으로 삭제 예약하도록 변경했습니다.
동아리 탈퇴/위드 탈퇴 시 작성한 게시글/댓글 파일과 프로필 파일을 30일 보관 삭제 예약하도록 처리했습니다.
Why
파일 교체/명시 삭제와 탈퇴로 인한 삭제는 보관 정책이 달라 즉시 정리 대상과 30일 보관 대상을 분리했습니다.
How
File.immediateHardDeleteAfter,File.retainedHardDeleteAfter로 즉시 정리/30일 보관 정책을 구분했습니다.📸 Screenshots / Logs
💡 Reviewer 참고사항
마이그레이션은 PR 리뷰 후 반영 예정입니다!
마이페이지를 수정할 예정이라서 동아리 탈퇴시엔 프로필 이미지 삭제 X, 위드 탈퇴시엔 프로필 이미지 삭제 예약으로 구현해뒀습니다.
파일 교체/명시 삭제는 삭제예약 = 즉시, 위드 탈퇴, 동아리 탈퇴 = 30일 예약을 해두었습니다.
✅ Checklist
Summary by CodeRabbit
New Features
Bug Fixes