[LNK-33] 이미지 리사이징 로직 수정 #78
Conversation
Walkthrough업로드된 미디어를 임시(temp) 위치에서 originals로 복사·이동하고 엔티티 URL을 갱신하도록 S3 관련 서비스와 미디어 업데이트 흐름을 여러 유스케이스에 통합했습니다(새 서비스/예외/설정 추가 포함). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Usecase
participant MediaSaveService
participant MediaS3Service
participant MediaUpdateService
participant S3PresignedUrlService
Client->>Usecase: uploadXXX(request)
Usecase->>MediaSaveService: save(medias)
MediaSaveService-->>Usecase: savedMedias
loop per savedMedia
Usecase->>MediaS3Service: moveToOriginals(tempUrl)
MediaS3Service->>S3PresignedUrlService: copyObject(tempKey, originalsKey)
S3PresignedUrlService-->>MediaS3Service: copied
MediaS3Service->>S3PresignedUrlService: deleteObject(tempKey)
S3PresignedUrlService-->>MediaS3Service: deleted
MediaS3Service->>S3PresignedUrlService: getUrl(originalsKey)
S3PresignedUrlService-->>MediaS3Service: originalsUrl
MediaS3Service-->>Usecase: originalsUrl
Usecase->>MediaUpdateService: updateMediaUrl(media, originalsUrl)
MediaUpdateService-->>Usecase: updated
end
Usecase-->>Client: completed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![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
♻️ Duplicate comments (1)
src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java (1)
260-263: 동일한 에러 처리 및 트랜잭션 일관성 문제가 존재합니다.Lines 185-188과 동일한 문제가 있습니다. S3 작업 실패 시 에러 처리와 트랜잭션 롤백 시 고아 파일 처리를 위한 로직이 필요합니다.
동일한 에러 처리 패턴을 적용하세요:
newMedias.forEach(media -> { - String originalsUrl = mediaS3Service.moveToOriginals(media.getMediaUrl()); - mediaUpdateService.updateMediaUrl(media, originalsUrl); + try { + String originalsUrl = mediaS3Service.moveToOriginals(media.getMediaUrl()); + mediaUpdateService.updateMediaUrl(media, originalsUrl); + } catch (Exception e) { + throw new MediaProcessingException("미디어 업데이트 중 오류 발생", e); + } });
🧹 Nitpick comments (4)
src/main/java/leets/leenk/global/config/AwsS3Config.java (1)
36-44: S3Client 빈 정의는 적절하지만 종료 시 자원 정리를 명시해 두면 더 안전합니다.
s3Presigner()와 동일한 방식으로 자격 증명과 리전을 설정한s3Client()빈 추가는 괜찮아 보입니다.
다만 AWS SDK v2S3Client는 내부적으로 커넥션 풀을 가지는 Closeable 객체라, 애플리케이션 종료 시@Bean(destroyMethod = "close")형태로 정리까지 명시해 두면 장기 실행 시 리소스 관리 측면에서 조금 더 안전합니다.src/main/java/leets/leenk/domain/leenk/application/usecase/LeenkUsecase.java (1)
73-151: Leenk 미디어 temp → originals 플로우 일관성은 좋고, URL 업데이트 방식만 약간 정리 여지가 있습니다.
uploadLeenk()에서 Media 저장 직후mediaS3Service.moveToOriginals()+mediaUpdateService.updateMediaUrl()로 최종 URL을 반영하고,updateLeenk()에서도 새 URL이 있는 경우 동일한 temp → originals 정책을 따르게 한 구조는 이번 PR 목표(임시 경로 사용 후 원본으로 이동)에 잘 맞습니다.다만
updateLeenk()에서는 기존 미디어/새 미디어 모두media.updateMediaUrl(...)를 직접 호출하고 있어,MediaUpdateService를 사용하는 경우와 엔티티 직접 호출이 혼재되어 있습니다. 도메인 수정 진입점을 서비스 레이어로 통일하고 싶다면, 여기서도mediaUpdateService.updateMediaUrl(...)를 사용하는 쪽으로 맞추는 리팩터를 한 번에 해 두어도 좋겠습니다(필수는 아님).Based on learnings
src/main/java/leets/leenk/domain/user/domain/service/user/UserUpdateService.java (1)
3-32: 회원 프로필 초기 설정도 temp → originals로 맞춘 점은 좋고, 의존성 변경 영향만 한 번 확인해 주세요.
completeProfile()에서mediaS3Service.moveToOriginals()를 사용해 가입 시 프로필 이미지도 temp 경로를 거쳐 originals로 정규화하는 흐름으로 맞춘 것은 다른 유즈케이스와의 일관성 측면에서 좋습니다.이제
UserUpdateService가MediaS3Service를 생성자 주입으로 요구하므로, 테스트나 설정 코드에서new UserUpdateService(...)처럼 수동 생성하던 부분이 있었다면 컴파일/런타임 에러가 없는지 한 번 점검해 보시는 게 좋겠습니다. 또한, 추후에updateProfileImage(User, String)에서도moveToOriginals호출을 캡슐화하도록 리팩터링하면, 프로필 이미지 업데이트 경로를 서비스 내부로 더 잘 숨길 수 있을 것 같습니다(이번 PR에서는 선택 사항).src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java (1)
95-98: URL 생성 시 region 및 bucket 값 검증을 고려하세요.현재 region과 bucket 값으로 URL을 생성하지만, 값이 올바른지 검증하지 않습니다. 설정 오류 시 잘못된 URL이 생성될 수 있습니다.
애플리케이션 시작 시 region과 bucket 값이 올바르게 설정되었는지 검증하는 로직을 추가하는 것을 고려하세요:
@PostConstruct public void validateConfiguration() { if (bucket == null || bucket.isBlank()) { throw new IllegalStateException("S3 bucket 설정이 없습니다"); } if (region == null || region.isBlank()) { throw new IllegalStateException("AWS region 설정이 없습니다"); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java(4 hunks)src/main/java/leets/leenk/domain/leenk/application/usecase/LeenkUsecase.java(4 hunks)src/main/java/leets/leenk/domain/media/domain/entity/Media.java(0 hunks)src/main/java/leets/leenk/domain/media/domain/service/MediaS3Service.java(1 hunks)src/main/java/leets/leenk/domain/media/domain/service/MediaUpdateService.java(1 hunks)src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java(3 hunks)src/main/java/leets/leenk/domain/user/application/usecase/UserUsecase.java(3 hunks)src/main/java/leets/leenk/domain/user/domain/service/user/UserUpdateService.java(2 hunks)src/main/java/leets/leenk/global/config/AwsS3Config.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/leets/leenk/domain/media/domain/entity/Media.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: hyxklee
Repo: Leets-Makers/Leenk-BE PR: 54
File: src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java:175-181
Timestamp: 2025-08-12T14:03:25.193Z
Learning: In the Leenk-BE project, for media updates in feeds, the team prefers complete deletion and recreation over differential updates to maintain data integrity, especially given the low-traffic nature of the service. Performance optimization is deprioritized in favor of data consistency and simpler logic.
📚 Learning: 2025-08-12T14:03:25.193Z
Learnt from: hyxklee
Repo: Leets-Makers/Leenk-BE PR: 54
File: src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java:175-181
Timestamp: 2025-08-12T14:03:25.193Z
Learning: In the Leenk-BE project, for media updates in feeds, the team prefers complete deletion and recreation over differential updates to maintain data integrity, especially given the low-traffic nature of the service. Performance optimization is deprioritized in favor of data consistency and simpler logic.
Applied to files:
src/main/java/leets/leenk/domain/leenk/application/usecase/LeenkUsecase.javasrc/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java
🧬 Code graph analysis (2)
src/main/java/leets/leenk/domain/media/domain/service/MediaS3Service.java (1)
src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java (1)
Service(21-100)
src/main/java/leets/leenk/domain/user/domain/service/user/UserUpdateService.java (1)
src/main/java/leets/leenk/domain/user/application/usecase/UserUsecase.java (1)
Service(25-144)
🔇 Additional comments (5)
src/main/java/leets/leenk/domain/media/domain/service/MediaUpdateService.java (1)
15-17: 미디어 URL 전용 업데이트 메서드 분리는 방향성 좋습니다.
update()와 대칭되는 형태로updateMediaUrl()을 분리해 둔 덕분에, 도메인 수정 진입점이 서비스 레이어에 모여 있어 이후 로직(이벤트 발행, 감사 로그 등)을 추가하기도 좋아 보입니다.src/main/java/leets/leenk/domain/user/application/usecase/UserUsecase.java (1)
41-77: 프로필 이미지도 temp → originals 플로우를 타도록 정렬된 점이 좋습니다.
updateProfileImage()에서 바로mediaS3Service.moveToOriginals()를 호출한 뒤, 정제된 URL만UserUpdateService에 넘기는 구조라 유즈케이스 레벨에서 S3 경로 정책이 명확해졌습니다.moveToOriginals()가 null/blank, 이미/originals/인 경우를 처리해 주기 때문에 호출 측에서 별도 분기 없이 사용하기에도 적절해 보입니다.src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java (1)
60-61: 새로운 서비스 의존성 추가가 적절합니다.temp → originals 플로우를 구현하기 위해 필요한 서비스들이 올바르게 주입되었습니다.
src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java (2)
72-72: 핵심 변경사항: temp 경로로 업로드됩니다.presigned URL이 이제
temp/경로를 가리키므로, 파일이 임시 위치에 먼저 업로드됩니다. 이는 PR 목표인 "Lambda 트리거 타이밍 제어"를 달성하기 위한 핵심 변경사항입니다.
27-27: S3Client 빈 설정이 올바르게 구현되어 있습니다.검증 결과,
AwsS3Config.java(Line 36-43)에서S3Client빈이@Bean메서드로 정의되어 있으며,AwsBasicCredentials와Region을 사용하여 올바르게 구성되어 있습니다.S3PresignedUrlService에서 이 빈을 주입받아 사용하고 있으므로 설정에 문제가 없습니다.
| medias.forEach(media -> { | ||
| String newMediaUrl = mediaS3Service.moveToOriginals(media.getMediaUrl()); | ||
| mediaUpdateService.updateMediaUrl(media, newMediaUrl); | ||
| }); |
There was a problem hiding this comment.
S3 작업의 에러 처리와 트랜잭션 일관성을 보장해야 합니다.
현재 구현에는 다음과 같은 문제가 있습니다:
- 에러 처리 부재:
moveToOriginals나updateMediaUrl이 실패해도 예외가 처리되지 않아 부분 실패 시 일관성이 깨질 수 있습니다. - 트랜잭션 경계 문제: S3 복사가 성공한 후 트랜잭션이 롤백되면 originals에 고아 파일이 남게 됩니다.
- 원자성 부족: forEach에서 하나의 미디어 처리가 실패해도 다른 미디어는 계속 처리되어 불일치 상태가 발생합니다.
다음 개선 방안을 고려하세요:
medias.forEach(media -> {
- String newMediaUrl = mediaS3Service.moveToOriginals(media.getMediaUrl());
- mediaUpdateService.updateMediaUrl(media, newMediaUrl);
+ try {
+ String newMediaUrl = mediaS3Service.moveToOriginals(media.getMediaUrl());
+ mediaUpdateService.updateMediaUrl(media, newMediaUrl);
+ } catch (Exception e) {
+ // 실패 시 이미 처리된 미디어들도 롤백하거나, 전체 작업 실패 처리
+ throw new MediaProcessingException("미디어 처리 중 오류 발생", e);
+ }
});추가로, S3 작업 실패 시 이미 복사된 파일들을 정리하는 보상 트랜잭션(compensating transaction) 로직도 고려해보세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java
around lines 185-188, replace the blind medias.forEach(...) with a safe,
transactional flow: perform S3 moves sequentially (use a for-loop) and collect
the successfully moved object keys; if any move fails, stop further processing,
delete any already moved objects (compensating cleanup) and rethrow the
exception; once all S3 moves succeed, open a DB transaction and perform all
mediaUpdateService.updateMediaUrl calls inside it; if the DB transaction fails
or is rolled back, catch the failure, delete the moved S3 objects (compensating
transaction), and rethrow or return an error; ensure all exceptions are logged
with details and avoid swallowing them so callers can handle retries or alerts.
There was a problem hiding this comment.
copy 작업 실패 시 예외 처리가 되고 Transaction어노테이션으로 전체 트랜잭션이 롤백되어서 일관성이 보장 돼
There was a problem hiding this comment.
@soo0711, 말씀하신 부분이 부분적으로 맞지만, 중요한 분산 트랜잭션 문제가 여전히 존재합니다.
@transactional의 한계:
@Transactional 어노테이션은 데이터베이스 트랜잭션만 제어합니다. S3와 같은 외부 시스템 작업은 Spring의 트랜잭션 관리 범위 밖에 있어서 롤백할 수 없습니다.
문제 시나리오:
medias.forEach(media -> {
// 1. S3 복사 성공 (temp → originals)
String newMediaUrl = mediaS3Service.moveToOriginals(media.getMediaUrl());
// 2. DB 업데이트 시도
mediaUpdateService.updateMediaUrl(media, newMediaUrl);
// 3. 이후 다른 DB 작업 실패 → 트랜잭션 롤백
});위 경우:
- ✅ S3에 파일은 이미 originals 경로로 복사됨
- ❌ DB 트랜잭션 롤백 → DB에는 새 URL이 저장되지 않음
⚠️ 결과: originals에 고아 파일 발생
개선 방안:
- 보상 트랜잭션 패턴: S3 작업 후 DB 실패 시 이미 복사한 파일들을 삭제
- 작업 순서 변경: DB 업데이트를 먼저 수행하고 커밋 후 S3 작업 (단, 다른 트레이드오프 존재)
- 재시도 메커니즘: 실패 시 전체 작업을 재시도하는 로직 추가
현재 구현에서 S3CopyException으로 인한 즉각적인 실패는 잘 처리되지만, S3 성공 후 DB 실패 시나리오에 대한 정리 로직이 필요합니다. 이 부분을 어떻게 처리할 계획이신가요?
🧠 Learnings used
Learnt from: hyxklee
Repo: Leets-Makers/Leenk-BE PR: 54
File: src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java:175-181
Timestamp: 2025-08-12T14:03:25.193Z
Learning: In the Leenk-BE project, for media updates in feeds, the team prefers complete deletion and recreation over differential updates to maintain data integrity, especially given the low-traffic nature of the service. Performance optimization is deprioritized in favor of data consistency and simpler logic.
There was a problem hiding this comment.
originals에 고아 파일이 남는 건 맞지만 서비스 동작에 영향 없을 것 같고 DB 커밋 실패는 매우 드물게 발생하기 때문에 보상 트랜잭션 구현 복잡도가 얻는 실익보다 크다고 생각해
There was a problem hiding this comment.
@soo0711, 합리적인 판단이라고 생각합니다. 말씀하신 대로 DB 커밋 실패는 매우 드물고, 고아 파일이 서비스 동작에는 영향을 주지 않으니 보상 트랜잭션의 복잡도가 실익을 초과할 수 있겠네요.
다만 장기적으로 고아 파일이 누적될 경우를 대비해, 필요하다면 추후 S3 라이프사이클 정책이나 주기적인 정리 스크립트로 처리하는 것도 고려해볼 수 있을 것 같습니다. 현재 구현은 이대로 괜찮아 보입니다! 👍
🧠 Learnings used
Learnt from: hyxklee
Repo: Leets-Makers/Leenk-BE PR: 54
File: src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java:175-181
Timestamp: 2025-08-12T14:03:25.193Z
Learning: In the Leenk-BE project, for media updates in feeds, the team prefers complete deletion and recreation over differential updates to maintain data integrity, especially given the low-traffic nature of the service. Performance optimization is deprioritized in favor of data consistency and simpler logic.
| public String moveToOriginals(String tempUrl) { | ||
| if (tempUrl == null || tempUrl.isBlank()) { | ||
| return tempUrl; | ||
| } | ||
|
|
||
| if (tempUrl.contains("/originals/")) { | ||
| return tempUrl; | ||
| } | ||
|
|
||
| String tempKey = extractKeyFromUrl(tempUrl); | ||
| String originalsKey = tempKey.replace("temp/", "originals/"); | ||
|
|
||
| s3PresignedUrlService.copyObject(tempKey, originalsKey); | ||
| s3PresignedUrlService.deleteObject(tempKey); | ||
|
|
||
| return s3PresignedUrlService.getUrl(originalsKey); | ||
| } | ||
|
|
||
| private String extractKeyFromUrl(String url) { | ||
| URI uri = URI.create(url); | ||
| return uri.getPath().substring(1); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
extractKeyFromUrl가 상대 경로나 비표준 URL을 받을 경우 키가 잘릴 수 있습니다.
현재 구현은 URI.create(url).getPath().substring(1)에 의존해서, 경로가 항상 /temp/...처럼 /로 시작한다고 가정하고 있습니다. 만약 실수로 temp/...와 같은 상대 경로나 스킴 없는 문자열이 넘어오면 getPath()가 "temp/..."를 반환해 "emp/..."로 잘려 잘못된 키로 S3 copy/delete를 시도하게 됩니다.
아래처럼 경로가 /로 시작하는 경우에만 substring(1)을 적용하도록 방어 코드를 두면, 예외적인 입력에도 더 안전합니다.
- private String extractKeyFromUrl(String url) {
- URI uri = URI.create(url);
- return uri.getPath().substring(1);
- }
+ private String extractKeyFromUrl(String url) {
+ URI uri = URI.create(url);
+ String path = uri.getPath();
+
+ if (path == null || path.isBlank()) {
+ return path;
+ }
+
+ return path.startsWith("/") ? path.substring(1) : path;
+ }현재 호출부들은 모두 정규화된 URL을 넘길 것으로 보이지만, 향후 유지보수나 재사용성을 고려하면 이 정도 방어는 해 두는 편이 좋겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all usages of moveToOriginals method
rg -nP 'moveToOriginals\s*\(' --type=java
Length of output: 1312
🏁 Script executed:
#!/bin/bash
# Find test files for MediaS3Service
fd -e java -p '**/test/**' -x rg -l 'MediaS3Service|moveToOriginals' {}Length of output: 289
🏁 Script executed:
#!/bin/bash
# Check the S3PresignedUrlService to understand URL format it returns
rg -nP -A 10 'getUrl\s*\(' --type=java | head -50Length of output: 3664
🏁 Script executed:
#!/bin/bash
# Look for any URL creation or S3 URL construction in the codebase
rg -nP 's3.*url|url.*s3' -i --type=java | head -30Length of output: 3222
---
🏁 Script executed:
```shell
#!/bin/bash
# Find Media entity to see how mediaUrl is stored/validated
rg -nP 'class Media\s*\{' -A 20 --type=java
Length of output: 1884
🏁 Script executed:
#!/bin/bash
# Check UserUpdateRequest to see profileImage field definition
rg -nP 'profileImage' --type=java -B 2 -A 2 | head -40Length of output: 3227
🏁 Script executed:
#!/bin/bash
# Find any test files for media or S3 related functionality
fd '\.java$' -path '*/test*' -type f | xargs rg -l 'moveToOriginals|extractKeyFromUrl' 2>/dev/nullLength of output: 550
🏁 Script executed:
#!/bin/bash
# Check if there's any URL validation logic before calling moveToOriginals
rg -nP -B 5 'moveToOriginals\(' --type=java | head -60Length of output: 5897
---
🏁 Script executed:
```shell
#!/bin/bash
# Check what format request.profileImage comes in - look for ProfileImageRequest DTO
rg -nP 'class ProfileImageRequest|record ProfileImageRequest' -A 5 --type=java
Length of output: 424
🏁 Script executed:
#!/bin/bash
# Check if URI.getPath() can return null and what substring(1) would do
cat > /tmp/test_uri.java << 'EOF'
import java.net.URI;
public class TestURI {
public static void main(String[] args) {
// Test case 1: absolute URL (S3)
String url1 = "https://bucket.s3.amazonaws.com/temp/file123";
URI uri1 = URI.create(url1);
String path1 = uri1.getPath();
System.out.println("URL: " + url1);
System.out.println("Path: " + path1);
System.out.println("After substring(1): " + (path1 != null ? path1.substring(1) : "NULL"));
System.out.println();
// Test case 2: relative path (potential bug)
String url2 = "temp/file123";
URI uri2 = URI.create(url2);
String path2 = uri2.getPath();
System.out.println("URL: " + url2);
System.out.println("Path: " + path2);
System.out.println("After substring(1): " + (path2 != null && path2.length() > 0 ? path2.substring(1) : "INVALID"));
System.out.println();
// Test case 3: just filename
String url3 = "file123";
URI uri3 = URI.create(url3);
String path3 = uri3.getPath();
System.out.println("URL: " + url3);
System.out.println("Path: " + path3);
System.out.println("After substring(1): " + (path3 != null && path3.length() > 0 ? path3.substring(1) : "INVALID"));
}
}
EOF
cd /tmp && javac test_uri.java && java TestURILength of output: 44
extractKeyFromUrl 메서드의 입력 검증 부족 - 방어 코드 추가 필요
현재 구현은 URI.create(url).getPath().substring(1)에 의존하면서, 경로가 항상 절대 경로 형식(예: /temp/...)을 따른다고 가정합니다. 만약 상대 경로나 형식이 잘못된 URL(예: temp/...)이 전달되면:
getPath()는"temp/..."를 반환substring(1)은 첫 문자를 제거하여"emp/..."생성 → S3 작업 실패 또는 데이터 손상
특히 request.profileImage()는 사용자 입력인데 현재 moveToOriginals 호출 전 형식 검증이 없습니다.
- private String extractKeyFromUrl(String url) {
- URI uri = URI.create(url);
- return uri.getPath().substring(1);
- }
+ private String extractKeyFromUrl(String url) {
+ URI uri = URI.create(url);
+ String path = uri.getPath();
+
+ if (path == null || path.isBlank()) {
+ return path;
+ }
+
+ return path.startsWith("/") ? path.substring(1) : path;
+ }현재 호출부에서 정규화된 S3 URL을 전달하지만, 향후 재사용성과 입력 안정성을 위해 방어 코드 추가를 권장합니다.
🤖 Prompt for AI Agents
In src/main/java/leets/leenk/domain/media/domain/service/MediaS3Service.java
around lines 15 to 36, the extractKeyFromUrl method assumes URI.getPath() always
returns an absolute path and blindly calls substring(1), which corrupts inputs
like "temp/..." or throws on invalid URIs; change it to be defensive: validate
input (null/blank already handled upstream), wrap URI.create in try/catch for
IllegalArgumentException/URISyntaxException, retrieve path and if path is null
treat the input as a raw key, only strip the leading '/' when
path.startsWith("/"), and for strings that are not full URLs (no scheme) treat
them as keys directly; ensure any parsing errors return or propagate a clear
exception rather than producing a malformed key.
| public void copyObject(String sourceKey, String destinationKey) { | ||
| CopyObjectRequest copyRequest = CopyObjectRequest.builder() | ||
| .sourceBucket(bucket) | ||
| .sourceKey(sourceKey) | ||
| .destinationBucket(bucket) | ||
| .destinationKey(destinationKey) | ||
| .build(); | ||
|
|
||
| s3Client.copyObject(copyRequest); | ||
| } |
There was a problem hiding this comment.
S3 복사 작업의 에러 처리가 필요합니다.
s3Client.copyObject()가 실패하면 SdkException을 던지지만, 현재 메서드는 이를 처리하지 않습니다. 호출자는 작업 성공 여부를 알 수 없어 불일치 상태가 발생할 수 있습니다.
다음과 같이 에러 처리를 추가하거나 명시적으로 예외를 전파하세요:
public void copyObject(String sourceKey, String destinationKey) {
- CopyObjectRequest copyRequest = CopyObjectRequest.builder()
- .sourceBucket(bucket)
- .sourceKey(sourceKey)
- .destinationBucket(bucket)
- .destinationKey(destinationKey)
- .build();
-
- s3Client.copyObject(copyRequest);
+ try {
+ CopyObjectRequest copyRequest = CopyObjectRequest.builder()
+ .sourceBucket(bucket)
+ .sourceKey(sourceKey)
+ .destinationBucket(bucket)
+ .destinationKey(destinationKey)
+ .build();
+
+ s3Client.copyObject(copyRequest);
+ } catch (SdkException e) {
+ throw new S3OperationException("S3 객체 복사 실패: " + sourceKey + " -> " + destinationKey, e);
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java
around lines 75 to 84, the copyObject method currently calls
s3Client.copyObject(...) without handling failures; add error handling by
catching SdkException (or AwsServiceException/SdkClientException as
appropriate), log the error with context (sourceKey, destinationKey, bucket) and
either rethrow it or wrap it in a domain-specific unchecked exception so callers
can react to failure; ensure you preserve the original exception as the cause
when rethrowing and avoid swallowing the exception so the caller can detect and
handle inconsistent state.
| public void deleteObject(String key) { | ||
| DeleteObjectRequest deleteRequest = DeleteObjectRequest.builder() | ||
| .bucket(bucket) | ||
| .key(key) | ||
| .build(); | ||
|
|
||
| s3Client.deleteObject(deleteRequest); | ||
| } |
There was a problem hiding this comment.
S3 삭제 작업의 에러 처리가 필요합니다.
copyObject와 마찬가지로 삭제 작업 실패 시 에러 처리가 없습니다. 삭제 실패 시 temp 파일이 남게 되어 스토리지 비용이 증가할 수 있습니다.
에러 처리를 추가하세요:
public void deleteObject(String key) {
- DeleteObjectRequest deleteRequest = DeleteObjectRequest.builder()
- .bucket(bucket)
- .key(key)
- .build();
-
- s3Client.deleteObject(deleteRequest);
+ try {
+ DeleteObjectRequest deleteRequest = DeleteObjectRequest.builder()
+ .bucket(bucket)
+ .key(key)
+ .build();
+
+ s3Client.deleteObject(deleteRequest);
+ } catch (SdkException e) {
+ // 삭제 실패는 로그만 남기고 계속 진행할 수도 있음 (temp 파일은 lifecycle policy로 정리 가능)
+ throw new S3OperationException("S3 객체 삭제 실패: " + key, e);
+ }
}또는 S3 버킷에 lifecycle policy를 설정하여 temp/ 폴더의 오래된 파일을 자동 삭제하는 방안도 고려하세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java
around lines 86-93: deleteObject currently calls
s3Client.deleteObject(deleteRequest) with no error handling; add try-catch
around the delete call to catch relevant AWS SDK exceptions (e.g., S3Exception,
AwsServiceException, SdkClientException or generic Exception), log the failure
with the key and exception details, and decide/implement a retry or compensating
action (e.g., mark temp file for later cleanup or enqueue a background cleanup
task); optionally, ensure callers receive a meaningful result/boolean or rethrow
a wrapped exception so higher layers can react; consider adding/mentioning a
lifecycle policy on the S3 bucket for automatic removal of old temp/ files as a
backup.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/leets/leenk/domain/media/application/exception/S3CopyException.java (1)
1-9: S3CopyException에 원인(cause) 전달용 생성자 추가를 고려해보면 좋겠습니다.현재 기본 생성자만 있어 S3 SDK에서 던지는 원본 예외를 체이닝하기 어렵습니다.
BaseException이 message/cause를 받는 생성자를 지원한다면, 아래처럼 오버로드를 추가해 두면 장애 시 추적이 훨씬 수월할 것 같습니다.public class S3CopyException extends BaseException { public S3CopyException() { super(ErrorCode.S3_COPY_FAILED); } public S3CopyException(Throwable cause) { super(ErrorCode.S3_COPY_FAILED, cause); } public S3CopyException(String message, Throwable cause) { super(ErrorCode.S3_COPY_FAILED, message, cause); } }필수는 아니고, 운영/디버깅 편의성을 위한 선택적 개선 포인트로 보입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/leets/leenk/domain/media/application/exception/ErrorCode.java(1 hunks)src/main/java/leets/leenk/domain/media/application/exception/S3CopyException.java(1 hunks)src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/leets/leenk/domain/media/domain/service/S3PresignedUrlService.java
🔇 Additional comments (1)
src/main/java/leets/leenk/domain/media/application/exception/ErrorCode.java (1)
13-18: S3 복사 실패 에러코드 추가는 일관성 있게 잘 정의되어 있습니다.미디어 도메인 250x 코드 체계,
INTERNAL_SERVER_ERROR매핑, 한글 메시지 모두 기존 스타일과 잘 맞습니다. 별도 수정 필요 없어 보입니다.
Related issue 🛠
작업 내용 💻
스크린샷 📷
같이 얘기해보고 싶은 내용이 있다면 작성 📢
원래 DB 등록 전 Lambda에서 대기를 주는 방법을 고려했지만
하여 임시 경로 방식을 적용했습니다.
혹시 더 나은 방법이나 개선 아이디어가 있다면 같이 논의해보고 싶습니다!
Summary by CodeRabbit
릴리스 노트
새 기능
개선사항