-
Notifications
You must be signed in to change notification settings - Fork 0
[WTH-415] 파일 삭제 구현 #83
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
base: dev
Are you sure you want to change the base?
The head ref may contain hidden characters: "feat/WTH-415-\uD30C\uC77C-\uC0AD\uC81C-\uAD6C\uD604"
Changes from all commits
c86c15c
73a9f8a
e85c62b
f8a2d13
5aa3d9e
78760a4
1c83c43
b6caa27
ab0c536
c2cccc1
176fdc2
2854991
9c24a45
02268e8
1e2b7aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,6 @@ import com.weeth.domain.club.domain.service.ClubJoinPolicy | |
| import com.weeth.domain.club.domain.service.ClubMemberPolicy | ||
| import com.weeth.domain.file.domain.entity.File | ||
| import com.weeth.domain.file.domain.enums.FileOwnerType | ||
| import com.weeth.domain.file.domain.enums.FileStatus | ||
| import com.weeth.domain.file.domain.port.FileAccessUrlPort | ||
| import com.weeth.domain.file.domain.repository.FileRepository | ||
| import com.weeth.domain.user.application.exception.UserInActiveException | ||
| import com.weeth.domain.user.domain.repository.UserReader | ||
|
|
@@ -47,7 +45,6 @@ class ManageClubMemberUsecase( | |
| private val clubJoinPolicy: ClubJoinPolicy, | ||
| private val clubActivityDeletionPolicy: ClubActivityDeletionPolicy, | ||
| private val fileRepository: FileRepository, | ||
| private val fileAccessUrlPort: FileAccessUrlPort, | ||
| private val clock: Clock, | ||
| ) { | ||
| /** | ||
|
|
@@ -92,19 +89,11 @@ class ManageClubMemberUsecase( | |
| userId: Long, | ||
| request: UpdateMemberProfileRequest, | ||
| ) { | ||
| val members = clubMemberRepository.findActiveByUserId(userId) | ||
| val members = clubMemberRepository.findAllActiveByUserIdWithLock(userId) | ||
| if (members.isEmpty()) throw ClubMemberNotFoundException() | ||
|
|
||
| request.profileImage?.let { profileImage -> | ||
| val existingFiles = | ||
| fileRepository.findAllByOwnerTypeAndOwnerIdAndStatus( | ||
| FileOwnerType.CLUB_MEMBER_PROFILE, | ||
| userId, | ||
| FileStatus.UPLOADED, | ||
| ) | ||
| if (existingFiles.isNotEmpty()) { | ||
| fileRepository.deleteAll(existingFiles) | ||
| } | ||
| markFilesDeleted(userId) | ||
|
|
||
| val file = | ||
| File.createUploaded( | ||
|
|
@@ -125,18 +114,10 @@ class ManageClubMemberUsecase( | |
|
|
||
| @Transactional | ||
| fun deleteProfileImage(userId: Long) { | ||
| val members = clubMemberRepository.findActiveByUserId(userId) | ||
| val members = clubMemberRepository.findAllActiveByUserIdWithLock(userId) | ||
| if (members.isEmpty()) throw ClubMemberNotFoundException() | ||
|
|
||
| val existingFiles = | ||
| fileRepository.findAllByOwnerTypeAndOwnerIdAndStatus( | ||
| FileOwnerType.CLUB_MEMBER_PROFILE, | ||
| userId, | ||
| FileStatus.UPLOADED, | ||
| ) | ||
| if (existingFiles.isNotEmpty()) { | ||
| fileRepository.deleteAll(existingFiles) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 근데 기존에 하드 딜리트를 하던 파트를 소프트 딜리트 + 즉시 정리 대상으로 표시하는 방향으로 수정된 이유가 있을까요??
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분은 DB와 S3 삭제 시점이 분리되어 있어서 소프트 딜리트 + 즉시 정리 대상으로 바꿨습니다! 하드 딜리트를 바로 해버리면 DB row는 삭제됐는데 S3 삭제가 실패하거나 반대로 S3를 먼저 삭제했는데 이후 트랜잭션이 롤백되면서 DB에는 파일 정보가 남아 있는데 S3는 없는 상태가 생길 수 있다고 생각했습니닷 그래서 API 호출 시점에는 hardDeleteAfter = now로 즉시 정리 대상임을 표시한 뒤 클린업 과정에서 S3 삭제 성공 후 하드 딜리트를 처리하는 것이 데이터 정합성 측면에서 더 적절하다고 생각해서 해당 방식으로 수정했습니다!! 이 방향에 대해서는 어떻게 생각하시나용?? |
||
| } | ||
| markFilesDeleted(userId) | ||
|
|
||
| members.forEach { it.removeProfileImage() } | ||
| } | ||
|
|
@@ -186,4 +167,17 @@ class ManageClubMemberUsecase( | |
| clubActivityDeletionPolicy.markMemberActivitiesDeleted(member, now) | ||
| member.leave(now) | ||
| } | ||
|
|
||
| /** | ||
| * 프로필 이미지 교체/명시 삭제는 복구 대상이 아니므로 즉시 정리 대상으로 표시 | ||
| */ | ||
| private fun markFilesDeleted(userId: Long) { | ||
| val now = LocalDateTime.now(clock) | ||
| fileRepository.markActiveDeletedByOwnerTypeAndOwnerId( | ||
| ownerType = FileOwnerType.CLUB_MEMBER_PROFILE, | ||
| ownerId = userId, | ||
| deletedAt = now, | ||
| hardDeleteAfter = File.immediateHardDeleteAfter(now), | ||
| ) | ||
| } | ||
| } | ||
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.
해당 메서드는 실제로 DB에서 삭제를 하진 않는 것으로 이해가 되는데, 위 API를 실제로 호출하게 되면 file의 삭제가 이루어지지 않아서 유니크 제약조건에서 fail이 발생할 수도 있을 것 같아요!
이전에 해당 문제가 발생했어서 명시적으로 flush를 해줬던건데 (delete가 insert 보다 늦게 동작하기 때문) 이 로직에서는 해당 문제는 없을까용??
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.
기존에는 delete가 insert보다 늦게 실행되면서 문제가 발생했는데 현재 로직에서는 삭제 마킹 update가 먼저 DB에 반영되도록 되어 있습니다!
말씀 주신 것처럼 소프트 딜리트라서 row는 남기 때문에 동일한 storageKey로 다시 insert되면 unique 제약에 걸릴 수는 있을 것 같습니당.. 하지만 현재 이미지 업로드 플로우에서는 UUID 기반으로 매번 새로운 storageKey를 생성하고 있어서 정상 플로우에서는 동일 storageKey가 재사용되지 않아 괜찮다고 판단했습니다!