feat: configure AWS S3 storage integration and env properties - #56
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughJWT 전용 예외를 공통 인증 오류로 통합했습니다. Lettuce Redis 연결과 파일 전용 비동기 실행기를 추가했습니다. S3 업로드·삭제 처리와 트랜잭션 후 파일 삭제 이벤트를 변경했습니다. API·배치·실시간 모듈의 저장소 설정을 조정했습니다. Changes인증 및 저장소 구성
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
momogo-core/src/main/java/com/momogo/core/common/exception/AuthErrorCode.java (1)
25-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win새 오류 코드의 응답 계약 테스트를 추가해 주세요.
JWT_SERIALIZATION_FAILED는AUTH-JWT_SERIALIZATION_FAILED와 HTTP 500을 오류 계약에 추가합니다.BusinessException과 전역 예외 처리기를 거치는 테스트에서 code, status, message를 확인해 주세요. 공통 오류 코드 통합의 회귀를 빠르게 확인할 수 있습니다.🤖 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 `@momogo-core/src/main/java/com/momogo/core/common/exception/AuthErrorCode.java` around lines 25 - 26, AuthErrorCode.JWT_SERIALIZATION_FAILED에 대한 오류 응답 계약 테스트를 추가하세요. BusinessException으로 해당 오류를 발생시키고 전역 예외 처리기를 통과시킨 뒤 응답의 code가 AUTH-JWT_SERIALIZATION_FAILED, status가 HTTP 500, message가 정의된 오류 메시지인지 검증하세요. 기존 공통 오류 코드 계약 테스트 패턴을 재사용해 통합 회귀를 확인하세요.momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java (1)
95-97: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift삭제 실패 처리의 계약을 명시하세요.
Line 95-97은 모든
Exception을 기록하고 정상 반환합니다. 호출자가 이후 데이터베이스 메타데이터를 삭제하면 S3 객체가 남을 수 있습니다.삭제가 best-effort라면 재시도, 정리 작업, 메트릭을 추가하세요. 삭제 실패가 호출자에게 전달되어야 한다면 공통
BusinessException으로 전파하고 예상 가능한 S3 SDK 예외만 처리하세요.StorageService.delete의 계약과 모든 호출자를 확인한 뒤 처리 방식을 결정하세요.🤖 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 `@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java` around lines 95 - 97, Review the contract of StorageService.delete and every caller of S3StorageService.delete, then ensure deletion failures cannot be silently treated as success. If failure must propagate, catch only expected S3 SDK exceptions and rethrow the project’s common BusinessException; if deletion is explicitly best-effort, add the required retry, cleanup, or metric handling instead of swallowing all Exception types.
🤖 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 `@momogo-api/src/main/java/com/momogo/api/auth/jwt/RedisJwtRegistry.java`:
- Around line 269-274: RedisJwtRegistry의 인터럽트 처리 경로에서 BusinessException 상세 메시지에
userId를 포함하지 말고 정적 문구만 전달하도록 수정하세요. userId는 기존 log.error 컨텍스트에만 유지하고,
LOCK_ACQUISITION_FAILED 예외의 메시지는 내부 사용자 식별자를 노출하지 않는 문구로 바꾸세요.
In `@momogo-api/src/main/resources/application.yaml`:
- Around line 115-119: Update the application.yaml storage configuration in
momogo-api/src/main/resources/application.yaml (lines 115-119),
momogo-batch/src/main/resources/application.yaml (lines 46-54), and
momogo-realtime/src/main/resources/application.yaml (lines 42-50): keep
app.storage.type defaulting to local, align the S3 bucket variable with
docker-compose.yml’s S3_BUCKET, and conditionally connect AWS variables and file
URL settings only when app.storage.type=s3. Ensure local storage does not
require AWS environment variables.
In
`@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java`:
- Around line 79-85: Update the key extraction logic around the fileUrl handling
to parse HTTP(S) URLs with java.net.URI and use only the URI path, excluding
query and fragment components. Preserve correct key extraction for
virtual-hosted, path-style, and CDN URLs, and add separate tests covering each
deletion-target URL form, including URLs with query parameters and fragments.
---
Nitpick comments:
In
`@momogo-core/src/main/java/com/momogo/core/common/exception/AuthErrorCode.java`:
- Around line 25-26: AuthErrorCode.JWT_SERIALIZATION_FAILED에 대한 오류 응답 계약 테스트를
추가하세요. BusinessException으로 해당 오류를 발생시키고 전역 예외 처리기를 통과시킨 뒤 응답의 code가
AUTH-JWT_SERIALIZATION_FAILED, status가 HTTP 500, message가 정의된 오류 메시지인지 검증하세요. 기존
공통 오류 코드 계약 테스트 패턴을 재사용해 통합 회귀를 확인하세요.
In
`@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java`:
- Around line 95-97: Review the contract of StorageService.delete and every
caller of S3StorageService.delete, then ensure deletion failures cannot be
silently treated as success. If failure must propagate, catch only expected S3
SDK exceptions and rethrow the project’s common BusinessException; if deletion
is explicitly best-effort, add the required retry, cleanup, or metric handling
instead of swallowing all Exception types.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: db66af92-08f6-4436-942c-03e71f6ba62f
📒 Files selected for processing (9)
momogo-api/src/main/java/com/momogo/api/auth/exception/JwtLockAcquisitionException.javamomogo-api/src/main/java/com/momogo/api/auth/exception/JwtSerializationException.javamomogo-api/src/main/java/com/momogo/api/auth/jwt/RedisJwtRegistry.javamomogo-api/src/main/resources/application.yamlmomogo-batch/src/main/resources/application.yamlmomogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.javamomogo-core/src/main/java/com/momogo/core/common/exception/AuthErrorCode.javamomogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.javamomogo-realtime/src/main/resources/application.yaml
💤 Files with no reviewable changes (2)
- momogo-api/src/main/java/com/momogo/api/auth/exception/JwtLockAcquisitionException.java
- momogo-api/src/main/java/com/momogo/api/auth/exception/JwtSerializationException.java
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.java (1)
44-87: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
SimpleAsyncTaskExecutor의setConcurrencyLimit은 요청 스레드를 블로킹합니다.
SimpleAsyncTaskExecutor는 한도 초과 슬롯을 큐에 보관하지 않고 호출 스레드가 작업 슬롯을 빌 때까지 대기하게 합니다. 현재@Async+@TransactionalEventListener(AFTER_COMMIT/AFTER_ROLLBACK)사용자는 트랜잭션 커밋 직후의 스레드에서 작업을 제출합니다.MAIL_EXECUTOR(20),USER_EXECUTOR(50),FILE_EXECUTOR(50)에서 슬롯이 가득 차면 트랜잭션 완료 흐름의 스레드가 블로킹됩니다.DEFAULT_EXECUTOR(100)도 같은 구조입니다. 요청 스레드 블로킹은 의도한 백프레셔가 아니라면 피해야 합니다.개정 제안:
- 무제한 실행이 가능하고 외부 의존성은 타임아웃 + 재시도 + 한정 수의
Thread::startVirtualThread/관된 스레드 풀로 보호하세요.- 의도한 한도가 필요하다면
SimpleAsyncTaskExecutor.setRejectTasksWhenLimitReached(true)로 실패를 즉시 전파하고 요청 처리 흐름에서 재시도/데드락 회피 로직이 없는지 확인하세요.🤖 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 `@momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.java` around lines 44 - 87, Update the executor configuration methods defaultExecutor, mailExecutor, userExecutor, and fileExecutor so concurrency limits do not block transaction/request threads. Either remove the SimpleAsyncTaskExecutor limits and enforce bounded concurrency in the external work paths, or enable immediate rejection with setRejectTasksWhenLimitReached(true) and ensure callers handle rejection without deadlocking; preserve each executor’s intended concurrency policy.
🧹 Nitpick comments (3)
momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java (2)
112-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win불필요한 로직:
contains("..")검사는 정규식과 중복됩니다.
DIRECTORY_PATTERN은^[a-zA-Z0-9/_-]+$로, 점(.) 문자를 애초에 허용하지 않습니다. 따라서directory.contains("..")체크는 도달할 수 없는 조건입니다.".."이 포함된 문자열은 이미 정규식 매칭에서 걸러집니다. 이 중복 검사는 읽는 사람에게 "정규식만으로는 부족하다"는 오해를 줄 수 있습니다.코드 지침에 따라 불필요한 로직은 제거하는 것이 좋습니다.
🧹 제안 리팩토링
private void validateDirectory(String directory) { - if (directory == null || directory.isBlank() || directory.contains("..") || !DIRECTORY_PATTERN.matcher(directory).matches()) { + if (directory == null || directory.isBlank() || !DIRECTORY_PATTERN.matcher(directory).matches()) { throw new BusinessException(GlobalErrorCode.INVALID_INPUT, "유효하지 않은 디렉토리 경로입니다."); } }정규식이 점(
.)을 허용하도록 나중에 변경된다면, 그때..상위 경로 방지 체크를 다시 추가해야 한다는 점을 주석으로 남겨두면 좋습니다.As per path instructions, "불필요한 로직... 확인"이 요구되어 해당 중복 검증을 지적합니다.
🤖 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 `@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java` around lines 112 - 116, Update validateDirectory to remove the redundant directory.contains("..") condition, since DIRECTORY_PATTERN already rejects periods; leave the null, blank, and pattern validations unchanged. Do not add additional logic or comments unless the pattern is later changed to allow periods.Source: Path instructions
136-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win신규 재시도/파싱 로직에 대한 테스트 추가를 권장합니다.
parseS3Key와executeDeleteWithRetry/handleRetryFailure는 순수 로직에 가까워 단위 테스트로 검증하기 좋은 대상입니다. PR 설명에 테스트가 아직 완료되지 않았다고 명시되어 있으므로, 다음 케이스를 우선 검토해 주세요.
parseS3Key: 순수 키, http(s) URL(쿼리/프래그먼트 포함), 잘못된 URL 형식, path-style/virtual-hosted-style URL.executeDeleteWithRetry: 4xx에서 즉시 중단, 5xx/SdkException에서 재시도 후 성공, 최대 재시도 소진 시 최종 실패 로그.테스트 코드 초안 작성을 도와드릴까요?
As per PR objectives, tests are not marked as completed for this PR.
🤖 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 `@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java` around lines 136 - 175, 추가된 S3 키 파싱 및 삭제 재시도 로직에 단위 테스트를 작성하세요. parseS3Key는 순수 키, 쿼리·프래그먼트를 포함한 HTTP(S) URL, 잘못된 URL, path-style 및 virtual-hosted-style URL을 검증하고, executeDeleteWithRetry는 4xx 즉시 중단, 5xx·SdkException 재시도 후 성공, 최대 재시도 소진 시 최종 실패 로그를 검증하도록 구성하세요.momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.java (1)
77-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win중복 코드: executor 생성 로직을 공통 메서드로 추출하세요.
defaultExecutor(라인 44-51),mailExecutor(56-63),userExecutor(68-75),fileExecutor(77-87)가 스레드 이름 접두사와concurrencyLimit값만 다르고 나머지 로직은 동일합니다. 이는 DRY 원칙 위반입니다. 새 executor 추가 시마다 동일한 4줄이 복제되어, 유지보수 시 실수 위험이 커집니다.공통 팩토리 메서드로 추출하면 가독성과 유지보수성이 좋아집니다.
♻️ 제안 리팩토링
+ private SimpleAsyncTaskExecutor createVirtualThreadExecutor(String threadNamePrefix, int concurrencyLimit) { + SimpleAsyncTaskExecutor executor = new SimpleAsyncTaskExecutor(threadNamePrefix); + executor.setVirtualThreads(true); + executor.setConcurrencyLimit(concurrencyLimit); + taskDecoratorProvider.ifAvailable(executor::setTaskDecorator); + return executor; + } + `@Bean`(name = FILE_EXECUTOR) public Executor fileExecutor() { - SimpleAsyncTaskExecutor executor = new SimpleAsyncTaskExecutor("file-async-"); - executor.setVirtualThreads(true); - executor.setConcurrencyLimit(50); - taskDecoratorProvider.ifAvailable(executor::setTaskDecorator); - return executor; + return createVirtualThreadExecutor("file-async-", 50); }같은 방식으로
defaultExecutor,mailExecutor,userExecutor도 정리할 수 있습니다.추가로,
100/20/50/50같은 동시성 한도 값은 하드코딩된 매직 넘버입니다.@Value로 외부 설정화하면 환경별(dev/prod) 튜닝이 쉬워집니다. 이 부분은 선택 사항입니다.🤖 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 `@momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.java` around lines 77 - 87, Extract the duplicated SimpleAsyncTaskExecutor setup from defaultExecutor, mailExecutor, userExecutor, and fileExecutor into one shared factory method that accepts the thread-name prefix and concurrency limit, while preserving taskDecoratorProvider configuration and virtual-thread behavior; update each bean method to delegate to it. Externalizing the limits with `@Value` is optional and not required for this refactor.
🤖 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.
Outside diff comments:
In `@momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.java`:
- Around line 44-87: Update the executor configuration methods defaultExecutor,
mailExecutor, userExecutor, and fileExecutor so concurrency limits do not block
transaction/request threads. Either remove the SimpleAsyncTaskExecutor limits
and enforce bounded concurrency in the external work paths, or enable immediate
rejection with setRejectTasksWhenLimitReached(true) and ensure callers handle
rejection without deadlocking; preserve each executor’s intended concurrency
policy.
---
Nitpick comments:
In `@momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.java`:
- Around line 77-87: Extract the duplicated SimpleAsyncTaskExecutor setup from
defaultExecutor, mailExecutor, userExecutor, and fileExecutor into one shared
factory method that accepts the thread-name prefix and concurrency limit, while
preserving taskDecoratorProvider configuration and virtual-thread behavior;
update each bean method to delegate to it. Externalizing the limits with `@Value`
is optional and not required for this refactor.
In
`@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java`:
- Around line 112-116: Update validateDirectory to remove the redundant
directory.contains("..") condition, since DIRECTORY_PATTERN already rejects
periods; leave the null, blank, and pattern validations unchanged. Do not add
additional logic or comments unless the pattern is later changed to allow
periods.
- Around line 136-175: 추가된 S3 키 파싱 및 삭제 재시도 로직에 단위 테스트를 작성하세요. parseS3Key는 순수 키,
쿼리·프래그먼트를 포함한 HTTP(S) URL, 잘못된 URL, path-style 및 virtual-hosted-style URL을 검증하고,
executeDeleteWithRetry는 4xx 즉시 중단, 5xx·SdkException 재시도 후 성공, 최대 재시도 소진 시 최종 실패
로그를 검증하도록 구성하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f24cded-e4e9-41e8-806c-5863317d5fae
📒 Files selected for processing (12)
momogo-api/src/main/java/com/momogo/api/auth/jwt/RedisJwtRegistry.javamomogo-api/src/main/resources/application.yamlmomogo-batch/src/main/resources/application.yamlmomogo-core/build.gradlemomogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.javamomogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.javamomogo-core/src/main/java/com/momogo/core/common/storage/event/FileDeleteEvent.javamomogo-core/src/main/java/com/momogo/core/common/storage/event/FileRollbackDeleteEvent.javamomogo-core/src/main/java/com/momogo/core/common/storage/listener/FileDeleteEventListener.javamomogo-core/src/main/java/com/momogo/core/domain/user/service/UserHardDeleteProcessor.javamomogo-core/src/main/java/com/momogo/core/domain/user/service/UserServiceImpl.javamomogo-realtime/src/main/resources/application.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- momogo-realtime/src/main/resources/application.yaml
- momogo-api/src/main/resources/application.yaml
- momogo-batch/src/main/resources/application.yaml
- momogo-api/src/main/java/com/momogo/api/auth/jwt/RedisJwtRegistry.java
Junkov0
left a comment
Junkov0
left a comment
There was a problem hiding this comment.
이상 없는 것 같습니다!
작성 하느라 수고많으셨습니다!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.java`:
- Line 18: createVirtualThreadExecutor에서 defaultExecutor, mailExecutor,
userExecutor, fileExecutor에 사용하는 SimpleAsyncTaskExecutor 각각에
setConcurrencyLimit(...)을 적용해 실행 중인 작업 수를 제한하세요. 저장소 외부 API 호출과 재시도, S3 client
connection pool을 고려한 명시적이고 운영 가능한 한도를 설정하고, 기존 비동기 실행 동작은 유지하세요.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aa3f6d77-c02c-4433-9136-ef8969dc6d92
📒 Files selected for processing (2)
momogo-core/src/main/java/com/momogo/core/common/config/AsyncConfig.javamomogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java
3 participants
작업 내용
변경 사항
체크리스트
참고 사항
관련 이슈
Summary by CodeRabbit
개선 사항
설정 변경