[Refactor] S3 잔재 완전 제거 및 GCS 정합성 맞추기#210
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAsset 엔티티의 S3 잔재 필드( ChangesS3 필드명 제거 및 GCS 정합성 통일
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/main/java/com/proovy/domain/asset/service/AssetsServiceImpl.java (1)
97-97: ⚡ Quick win변경한 경로에서는 로컬 이름도
objectKey기준으로 맞춰 주세요.여기서는 이미
getObjectKey()/getThumbnailObjectKey()를 쓰고 있는데 로컬 변수명이 아직s3Key,thumbnailS3Key입니다. 이번 PR 목표가 S3 잔재 제거라면, 최소한 수정한 라인에서는 이름도 같이 정리해 두는 편이 이후 검색/추적에서 덜 헷갈립니다.♻️ Suggested cleanup
- .objectKey(s3Key) + .objectKey(objectKey) ... - final String s3Key = asset.getObjectKey(); + final String objectKey = asset.getObjectKey(); ... - final String s3Key = asset.getObjectKey(); - final String thumbnailS3Key = asset.getThumbnailObjectKey(); + final String objectKey = asset.getObjectKey(); + final String thumbnailObjectKey = asset.getThumbnailObjectKey();Also applies to: 220-220, 296-297
🤖 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/java/com/proovy/domain/asset/service/AssetsServiceImpl.java` at line 97, Rename the local variables `s3Key` and `thumbnailS3Key` to `objectKey` and `thumbnailObjectKey` respectively in AssetsServiceImpl where you already call `getObjectKey()` / `getThumbnailObjectKey()` so local names match the accessor names; update all usages in the method(s) (e.g., the builder call `.objectKey(s3Key)` and analogous thumbnail usage) to use the new variable names to keep naming consistent and remove S3-specific remnants.src/main/java/com/proovy/domain/storage/service/StorageService.java (1)
69-77: ⚡ Quick win벌크 삭제에서도 같은 object key를 두 번 넣을 수 있습니다.
썸네일이 원본 fallback인 asset이면 여기서
objectKey와thumbnailObjectKey가 동일해질 수 있습니다. 삭제 대상에는 원본과 다른 썸네일 key만 넣어 중복 호출을 막는 편이 좋겠습니다.♻️ Suggested fix
for (Asset asset : assets) { // 원본 파일 s3KeysToDelete.add(asset.getObjectKey()); totalFileSize += asset.getFileSize(); // 썸네일 파일 - if (asset.getThumbnailObjectKey() != null) { + if (asset.getThumbnailObjectKey() != null + && !asset.getThumbnailObjectKey().equals(asset.getObjectKey())) { s3KeysToDelete.add(asset.getThumbnailObjectKey()); } }🤖 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/java/com/proovy/domain/storage/service/StorageService.java` around lines 69 - 77, In StorageService (inside the loop that builds s3KeysToDelete), avoid adding duplicate S3 keys by skipping the thumbnail when asset.getThumbnailObjectKey() equals asset.getObjectKey(); alternatively replace the collection with a Set (e.g., use a HashSet for s3KeysToDelete) so add(asset.getThumbnailObjectKey()) will be a no-op for duplicates — ensure totalFileSize still sums original asset.getFileSize() only once and that downstream code expecting a list handles conversion from the Set if needed.src/main/java/com/proovy/domain/user/service/UserService.java (1)
208-216: ⚡ Quick win회원 탈퇴 후처리도 동일 key 중복 삭제를 피하는 게 좋습니다.
썸네일 fallback으로
thumbnailObjectKey == objectKey인 asset이 있으면afterCommit에서 같은 파일을 연속으로 삭제하게 됩니다. 여기서도 썸네일 key는 원본과 다를 때만 수집해 두는 편이 안전합니다.♻️ Suggested fix
assetRepository.findAllByUserId(userId).forEach(asset -> { if (asset.getObjectKey() != null) { objectKeys.add(asset.getObjectKey()); } - if (asset.getThumbnailObjectKey() != null) { + if (asset.getThumbnailObjectKey() != null + && !asset.getThumbnailObjectKey().equals(asset.getObjectKey())) { objectKeys.add(asset.getThumbnailObjectKey()); } });Also applies to: 233-239
🤖 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/java/com/proovy/domain/user/service/UserService.java` around lines 208 - 216, The current collection loop in assetRepository.findAllByUserId(...) collects both asset.getObjectKey() and asset.getThumbnailObjectKey() unconditionally, which can cause duplicate deletes when thumbnailObjectKey equals objectKey (e.g., during afterCommit deletion). Change the collection logic so you only add thumbnailObjectKey when it is non-null AND not equal to asset.getObjectKey(); ensure the same de-duplication is applied in the other block mentioned (lines 233-239) and anywhere afterCommit performs deletes using the objectKeys list to avoid double-deleting the same GCS key.src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java (1)
367-374: ⚡ Quick win원본 fallback 썸네일까지 그대로 넣으면 삭제 키가 중복됩니다.
confirmUpload()쪽에서 썸네일 생성 실패 시thumbnailObjectKey를 원본objectKey로 fallback 할 수 있어서, 여기서는 같은 asset의 key가 두 번 수집될 수 있습니다. 썸네일 key는 원본과 다를 때만 추가해 두는 편이 안전합니다.♻️ Suggested fix
for (Asset asset : assets) { if (asset.getObjectKey() != null) { s3KeysToDelete.add(asset.getObjectKey()); freedStorageBytes += asset.getFileSize(); } - if (asset.getThumbnailObjectKey() != null) { + if (asset.getThumbnailObjectKey() != null + && !asset.getThumbnailObjectKey().equals(asset.getObjectKey())) { s3KeysToDelete.add(asset.getThumbnailObjectKey()); } }🤖 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/java/com/proovy/domain/note/service/NoteServiceImpl.java` around lines 367 - 374, In NoteServiceImpl inside the loop that collects s3KeysToDelete and freedStorageBytes, avoid adding the thumbnail key when it equals the original object key (because confirmUpload may have fallen back thumbnailObjectKey to objectKey); change the logic that currently unconditionally adds asset.getThumbnailObjectKey() to only add it when thumbnailObjectKey is non-null AND not equal to asset.getObjectKey(), ensuring s3KeysToDelete and freedStorageBytes are not duplicated for the same asset.
🤖 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.
Nitpick comments:
In `@src/main/java/com/proovy/domain/asset/service/AssetsServiceImpl.java`:
- Line 97: Rename the local variables `s3Key` and `thumbnailS3Key` to
`objectKey` and `thumbnailObjectKey` respectively in AssetsServiceImpl where you
already call `getObjectKey()` / `getThumbnailObjectKey()` so local names match
the accessor names; update all usages in the method(s) (e.g., the builder call
`.objectKey(s3Key)` and analogous thumbnail usage) to use the new variable names
to keep naming consistent and remove S3-specific remnants.
In `@src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java`:
- Around line 367-374: In NoteServiceImpl inside the loop that collects
s3KeysToDelete and freedStorageBytes, avoid adding the thumbnail key when it
equals the original object key (because confirmUpload may have fallen back
thumbnailObjectKey to objectKey); change the logic that currently
unconditionally adds asset.getThumbnailObjectKey() to only add it when
thumbnailObjectKey is non-null AND not equal to asset.getObjectKey(), ensuring
s3KeysToDelete and freedStorageBytes are not duplicated for the same asset.
In `@src/main/java/com/proovy/domain/storage/service/StorageService.java`:
- Around line 69-77: In StorageService (inside the loop that builds
s3KeysToDelete), avoid adding duplicate S3 keys by skipping the thumbnail when
asset.getThumbnailObjectKey() equals asset.getObjectKey(); alternatively replace
the collection with a Set (e.g., use a HashSet for s3KeysToDelete) so
add(asset.getThumbnailObjectKey()) will be a no-op for duplicates — ensure
totalFileSize still sums original asset.getFileSize() only once and that
downstream code expecting a list handles conversion from the Set if needed.
In `@src/main/java/com/proovy/domain/user/service/UserService.java`:
- Around line 208-216: The current collection loop in
assetRepository.findAllByUserId(...) collects both asset.getObjectKey() and
asset.getThumbnailObjectKey() unconditionally, which can cause duplicate deletes
when thumbnailObjectKey equals objectKey (e.g., during afterCommit deletion).
Change the collection logic so you only add thumbnailObjectKey when it is
non-null AND not equal to asset.getObjectKey(); ensure the same de-duplication
is applied in the other block mentioned (lines 233-239) and anywhere afterCommit
performs deletes using the objectKeys list to avoid double-deleting the same GCS
key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92aa0870-22ce-40c2-b769-b64b4a4d7c46
📒 Files selected for processing (11)
src/main/java/com/proovy/domain/asset/dto/response/UploadConfirmResponse.javasrc/main/java/com/proovy/domain/asset/entity/Asset.javasrc/main/java/com/proovy/domain/asset/service/AssetsServiceImpl.javasrc/main/java/com/proovy/domain/conversation/dto/response/CanvasImageUploadResponse.javasrc/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.javasrc/main/java/com/proovy/domain/conversation/service/ConversationQueryServiceImpl.javasrc/main/java/com/proovy/domain/note/service/NoteServiceImpl.javasrc/main/java/com/proovy/domain/storage/service/StorageService.javasrc/main/java/com/proovy/domain/user/service/UserService.javasrc/main/resources/db/migration/V32__rename_s3_columns_to_object_keys.sqlsrc/test/java/com/proovy/domain/storage/service/StorageServiceTest.java
chowon442
left a comment
There was a problem hiding this comment.
이거 누가 작업한거죠? 코드래빗이 할 말을 잃어버렸네요... 🐇🐇🐇
₍ᐢ..ᐢ₎♡̷̷̷ ༘☆ 코드가은 |
📌 관련 이슈
🏷️ PR 타입
📝 작업 내용
S3 잔재 완전 제거 및 GCS 정합성 맞추기
AWS S3에서 GCP GCS로 마이그레이션 완료 후 남아있는 S3 잔재를 완전히 제거하고, DB 스키마와 엔티티간 정합성을 맞춰 prod 배포 시 Schema validation 오류를 해결했습니다.
주요 변경사항:
s3_key→object_key,thumbnail_s3_key→thumbnail_object_keys3Key→objectKey,thumbnailS3Key→thumbnailObjectKey@Column(name="...")추가수정된 파일들:
📸 스크린샷
빌드 성공 확인:
./gradlew clean build통과11개 파일 변경, 1개 마이그레이션 파일 생성
✅ 체크리스트
📎 기타 참고사항
ddl-auto: validate와 100% 호환되도록 수정Summary by CodeRabbit
릴리스 노트