Skip to content

feat: AWS S3 기반 파일 관리 시스템 구현#69

Merged
ysw789 merged 16 commits intodevfrom
feat/alt-149
Mar 9, 2026
Merged

feat: AWS S3 기반 파일 관리 시스템 구현#69
ysw789 merged 16 commits intodevfrom
feat/alt-149

Conversation

@ysw789
Copy link
Contributor

@ysw789 ysw789 commented Mar 7, 2026

관련 문서

https://www.notion.so/AWS-S3-31786553162880d88aa0f5d01271d9fc?source=copy_link

Summary by CodeRabbit

릴리스 노트

  • New Features

    • 파일 업로드, 프리사인드 URL(임시 접근) 조회 및 파일 삭제 기능 추가
    • 관리자·매니저 전용 파일 관리 API와 권한 기반 접근 제공
    • 프리사인드 URL 캐싱으로 임시 링크 재사용 및 성능 향상
    • 파일 유효성 검사(허용 형식, 20MB 제한), 상태 관리(미첨부→첨부→삭제) 및 오브젝트 스토리지 연동
    • 오래된 미사용 파일 자동 정리 스케줄러 추가
  • Chores

    • AWS S3 연동 관련 라이브러리 및 유틸리티 의존성 업데이트

ysw789 added 5 commits March 7, 2026 14:35
- AWS S3 SDK, UUID Generator 의존성 추가
- S3 버킷 및 Presigned URL 설정 추가
- multipart 파일 업로드 크기 제한 설정 (20MB)
- File 엔티티 정의 (UUID v7, PENDING→ATTACHED→DELETED 생명주기)
- FileTargetType, BucketType, FileStatus 열거형 추가
- 파일 관련 ErrorCode 추가 (B021-B025, C003-C004)
- 파일 유효성 검사 유틸리티(FileValidator) 추가
- S3ClientImpl: AWS S3 파일 업로드/삭제/Presigned URL 생성
- FileRepositoryImpl, FileQueryRepositoryImpl: JPA 기반 파일 영속성 처리
- 파일 업로드/삭제/Presigned URL 조회 UseCase (Admin, Manager 분리)
- AttachFiles: 파일을 대상 엔티티에 연결
- CleanupOrphanFiles: PENDING 상태 24시간 초과 파일 자동 삭제
- FileScheduleServiceImpl: 매일 새벽 3시 고아 파일 정리 스케줄러
- Admin/Manager 파일 업로드, Presigned URL 조회, 삭제 엔드포인트 구현
- OpenAPI 스펙 인터페이스(Spec) 분리
@ysw789 ysw789 requested review from hodoon and juny0955 March 7, 2026 05:40
@ysw789 ysw789 self-assigned this Mar 7, 2026
@ysw789 ysw789 added the FEAT label Mar 7, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 7, 2026

📝 Walkthrough

Walkthrough

파일 관리(업로드, 프리사인 URL 발급, 삭제) 기능 및 관련 도메인 모델, 포트/어댑터, 서비스, Redis 캐시, S3 연동 구성, 스케줄러, 예외/유틸리티, 빌드·설정 변경이 추가되었습니다.

Changes

Cohort / File(s) Summary
빌드·설정
build.gradle, src/main/resources/application.yml, src/test/resources/application.yml, src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java
AWS S3 SDK 및 UUID 의존성 추가/업데이트, multipart 제한 및 S3 설정(리전·버킷·프리사인 만료)과 S3Client/S3Presigner 빈 추가.
컨트롤러 / API 스펙
src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/..., src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/...
Admin/Manager용 파일 컨트롤러 및 Spec 인터페이스(업로드/프리사인 URL/삭제) 추가.
응답 DTO
src/main/java/com/dreamteam/alter/adapter/inbound/.../dto/*
AdminUploadFileResponseDto, ManagerUploadFileResponseDto, FileResponseDto, FilePresignedUrlResponseDto 등 DTO와 팩토리 메서드 추가.
도메인 모델 / 타입
src/main/java/com/dreamteam/alter/domain/file/...
File JPA 엔티티(생성·첨부·삭제 메서드), PresignedUrlResult 레코드, BucketType/FileStatus/FileTargetType 열거형 추가.
포트(인바운드/아웃바운드)
src/main/java/com/dreamteam/alter/domain/file/port/...
Admin/Manager/AttachFiles/Cleanup 등 인바운드 유스케이스 인터페이스와 FileQueryRepository, FileRepository, PresignedUrlCacheRepository, S3Client 등의 아웃바운드 포트 추가.
애플리케이션 서비스
src/main/java/com/dreamteam/alter/application/file/*
FileUploadService, FileUrlService, FileDeleteService 추가 — 파일 검증, S3 업로드/프리사인 조회/캐싱, 삭제·캐시 정리 흐름 구현.
유스케이스 구현
src/main/java/com/dreamteam/alter/application/file/usecase/*
Admin/Manager Upload/GetPresignedUrl/Delete, AttachFiles, CleanupOrphanFiles 등 구체 구현체 추가(권한·검증·트랜잭션 포함).
영속성 구현
src/main/java/com/dreamteam/alter/adapter/outbound/file/persistence/*
FileJpaRepository, FileRepositoryImpl, FileQueryRepositoryImpl(QueryDSL 기반) 추가.
외부 S3 구현
src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java
S3 업로드·프리사인 URL 생성·삭제 구현(버킷 매핑, 만료, 예외 매핑).
프리사인 URL 캐시(Redis)
src/main/java/com/dreamteam/alter/adapter/outbound/file/redis/*
PresignedUrlCacheEntry 레코드 및 PresignedUrlCacheRepositoryImpl(직렬화·TTL 계산·삭제) 추가.
스케줄러
src/main/java/com/dreamteam/alter/adapter/inbound/general/file/scheduler/FileCleanupScheduler.java
매일 03:00에 고아 파일 정리 스케줄러 추가(분산 잠금 사용).
예외·유틸리티
src/main/java/com/dreamteam/alter/common/exception/ErrorCode.java, src/main/java/com/dreamteam/alter/common/util/FileValidator.java, src/main/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandler.java
파일 관련 ErrorCode 7개 추가, 파일 검증·키 생성 유틸리티, Multipart 누락 예외 핸들러 추가.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller as Admin/Manager<br/>Controller
    participant Service as FileUploadService
    participant Validator as FileValidator
    participant S3 as S3ClientImpl
    participant Repo as FileRepository
    participant DB as Database

    Client->>Controller: POST /files (MultipartFile, targetType, bucketType)
    Controller->>Service: upload(file, targetType, bucketType, userId)
    Service->>Validator: validate(file)
    alt invalid
        Validator-->>Service: CustomException
        Service-->>Controller: 예외 전파
        Controller-->>Client: 400/413
    else valid
        Validator-->>Service: OK
    end
    Service->>Validator: sanitizeFileName / extractExtension / buildStoredKey
    Service->>S3: upload(file, storedKey, bucketType)
    alt S3 실패
        S3-->>Service: FILE_UPLOAD_FAILED 예외
        Service-->>Controller: 예외 전파
        Controller-->>Client: 500
    else 업로드 성공
        S3-->>Service: fileUrl
    end
    Service->>Repo: save(File)
    Repo->>DB: INSERT
    DB-->>Repo: created File
    Repo-->>Service: File(id)
    Service-->>Controller: fileId
    Controller-->>Client: 200 {fileId}
Loading
sequenceDiagram
    participant Client
    participant Controller as Admin/Manager<br/>Controller
    participant Repo as FileQueryRepository
    participant DB as Database
    participant Service as FileUrlService
    participant Cache as PresignedUrlCacheRepository
    participant S3 as S3ClientImpl

    Client->>Controller: GET /files/{fileId}/presigned-url
    Controller->>Repo: findById(fileId)
    Repo->>DB: SELECT
    alt not found
        DB-->>Repo: empty
        Repo-->>Controller: empty
        Controller-->>Client: 404 FILE_NOT_FOUND
    else found
        DB-->>Repo: File
        Repo-->>Controller: File
        Controller->>Service: getPresignedUrl(File)
        Service->>Cache: findByFileId(fileId)
        alt cache hit
            Cache-->>Service: PresignedUrlResult
            Service-->>Controller: result
        else cache miss
            Service->>S3: getPresignedUrl(storedKey, bucketType)
            S3-->>Service: PresignedUrlResult(url, expiresAt)
            Service->>Cache: save(fileId, result)
            Service-->>Controller: result
        end
        Controller-->>Client: 200 {presignedUrl, expiresAt}
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목이 주요 변경 사항을 정확하게 반영하고 있습니다. AWS S3 기반 파일 관리 시스템 구현이라는 제목은 추가된 모든 파일 업로드, 다운로드, 삭제 기능 및 관련 인프라와 일치합니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/alt-149

Comment @coderabbitai help to get the list of available commands and usage tips.

ysw789 added 3 commits March 7, 2026 15:58
- PresignedUrlResult 도메인 레코드 추가 (URL + 만료시각)
- Redis 기반 Presigned URL 캐시 구현 (PresignedUrlCacheRepository)
- FileUrlService로 URL 조회 로직 통합 (캐시 우선 조회)
- S3Client.getPresignedUrl 반환 타입을 PresignedUrlResult로 변경
- 응답 DTO에 expiresAt 필드 추가
- 공통 FileResponseDto 추가
- FileDeleteService로 S3 삭제 + 상태 변경 + 캐시 무효화 로직 통합
- AdminDeleteFile, ManagerDeleteFile에서 FileDeleteService 사용
- FileScheduleService 인터페이스 제거 (불필요한 추상화)
- FileScheduleServiceImpl → FileCleanupScheduler로 클래스명 변경
@ysw789 ysw789 marked this pull request as ready for review March 7, 2026 07:01
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 35

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/resources/application.yml (1)

75-83: ⚠️ Potential issue | 🟠 Major

YAML 플레이스홀더에 기본값이 없어 기본 자격증명 체인이 비활성화됩니다.

AwsS3ConfigAwsSesConfig는 이미 @Value("${aws.access-key:}") 방식으로 기본값을 설정하고 있어서, 자격증명이 없을 때 AWS SDK 기본 credential chain을 사용하도록 구현돼 있습니다. 그러나 application.yml의 ${AWS_ACCESS_KEY_ID}${AWS_SECRET_ACCESS_KEY}는 기본값이 없어서, Spring이 YAML을 로드하는 단계에서 이 환경변수들을 찾지 못하면 애플리케이션이 시작되지 않습니다.

이를 수정하려면 application.yml의 두 플레이스홀더에 기본값을 추가하세요:

설정 예시
 aws:
   region: ${AWS_REGION}
-  access-key: ${AWS_ACCESS_KEY_ID}
-  secret-key: ${AWS_SECRET_ACCESS_KEY}
+  access-key: ${AWS_ACCESS_KEY_ID:}
+  secret-key: ${AWS_SECRET_ACCESS_KEY:}
   s3:
     region: ${AWS_S3_REGION}
     public-bucket: ${AWS_S3_PUBLIC_BUCKET}
     private-bucket: ${AWS_S3_PRIVATE_BUCKET}
     presigned-url-expiration-minutes: ${AWS_S3_PRESIGNED_URL_EXPIRATION_MINUTES}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/application.yml` around lines 75 - 83, The YAML
placeholders for AWS credentials should include empty defaults so Spring doesn't
fail when env vars are absent; update the application.yml entries that use
${AWS_ACCESS_KEY_ID} and ${AWS_SECRET_ACCESS_KEY} to include default empty
values (e.g. ${AWS_ACCESS_KEY_ID:} and ${AWS_SECRET_ACCESS_KEY:}) so AwsS3Config
and AwsSesConfig (which rely on
`@Value`("${aws.access-key:}")/@Value("${aws.secret-key:}")) can fall back to the
AWS SDK default credential chain instead of preventing app startup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@build.gradle`:
- Around line 51-52: The AWS SDK S3 dependency version is outdated and pulls
vulnerable transitive dependencies; run ./gradlew dependencies to verify the
vulnerable artifacts are on the runtime classpath, then update the dependency
declaration for 'software.amazon.awssdk:s3:2.29.46' to
'software.amazon.awssdk:s3:2.34.0' and also update any SES AWS SDK dependency to
2.34.0 to keep versions consistent; after changing the version, re-run the
dependency report and run your test suite or a dependency scanner to confirm the
CVEs (CVE-2025-48924, CVE-2024-47554, CVE-2026-24400, CVE-2025-68161) are no
longer present on the classpath.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileController.java`:
- Around line 21-25: Add Lombok `@Getter` to the AdminFileController class so it
follows the controller guideline of using `@Getter` and `@RequiredArgsConstructor`
(no `@Setter`); update the annotation list on AdminFileController to include
`@Getter` alongside the existing `@RestController`, `@RequestMapping`, `@PreAuthorize`,
and `@RequiredArgsConstructor`, and ensure there are no `@Setter` annotations
present.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileController.java`:
- Around line 21-25: The controller class ManagerFileController is missing the
mandated Lombok `@Getter`; update the class declaration to include `@Getter`
alongside the existing `@RequiredArgsConstructor` (and keep no `@Setter`) so it
follows the controller guideline "Controllers use `@Getter`,
`@RequiredArgsConstructor` (no `@Setter`)"; ensure the Lombok import for Getter is
added if missing and that only ManagerFileController (the class implementing
ManagerFileControllerSpec) is changed.
- Around line 27-34: Replace the ambiguous field-level `@Resource` injection with
an explicit constructor that injects the three use cases:
ManagerUploadFileUseCase (managerUploadFile), ManagerGetPresignedUrlUseCase
(managerGetPresignedUrl) and ManagerDeleteFileUseCase (managerDeleteFile);
remove the `@Resource` annotations (and `@RequiredArgsConstructor` if present), add
a constructor in ManagerFileController that takes these three parameters and
annotate each constructor parameter with `@Qualifier`("managerUploadFile"),
`@Qualifier`("managerGetPresignedUrl") and `@Qualifier`("managerDeleteFile")
respectively (or use `@Autowired` on the constructor and `@Qualifier` on params) so
the intended beans are unambiguous and assigned to the final fields.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/dto/ManagerGetPresignedUrlResponseDto.java`:
- Around line 10-24: The ManagerGetPresignedUrlResponseDto class duplicates
another Presigned URL response DTO; replace the duplicated mapping by extracting
a shared DTO or shared mapper method: create a common PresignedUrlResponseDto
(or a utility mapper class) and move the mapping logic currently in
ManagerGetPresignedUrlResponseDto.of(PresignedUrlResult) into a single shared
factory (e.g., PresignedUrlResponseDto.of(PresignedUrlResult) or
PresignedUrlMappers.toResponse(PresignedUrlResult)), then update
ManagerGetPresignedUrlResponseDto to either extend/alias that common DTO or
delegate its of(...) to the shared factory so both manager and other consumers
use the same field/schema definitions and serialization rules.

In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java`:
- Around line 42-77: The upload() and getPresignedUrl() methods currently don't
consistently wrap AWS SDK/runtime exceptions: catch broad runtime I/O and AWS
SDK exceptions around the body of upload() (not just IOException) and around the
presign call in getPresignedUrl(), log the error with the storedKey and bucket,
and rethrow a CustomException with the appropriate ErrorCode (use
ErrorCode.FILE_UPLOAD_FAILED for upload() and ErrorCode.FILE_DELETE_FAILED if
matching delete() semantics or add a new FILE_PRESIGN_FAILED if more
appropriate) so their exception behavior matches delete(); locate these changes
in S3ClientImpl.upload and S3ClientImpl.getPresignedUrl (also mirror logging
pattern used in delete()).

In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/redis/PresignedUrlCacheEntry.java`:
- Line 4: Remove the unused JsonCreator import: the record
PresignedUrlCacheEntry does not use `@JsonCreator` (Jackson handles records with
`@JsonProperty` on components), so delete the line importing
com.fasterxml.jackson.annotation.JsonCreator and keep any needed `@JsonProperty`
annotations on the record components if present.

In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/redis/PresignedUrlCacheRepositoryImpl.java`:
- Around line 29-56: The Redis calls in findByFileId, save, and deleteByFileId
are not resilient: wrap redisTemplate.opsForValue().get(KEY_PREFIX + fileId),
redisTemplate.opsForValue().set(...), and redisTemplate.delete(KEY_PREFIX +
fileId) in try-catch that catches DataAccessException (or Exception) so Redis
failures are treated as best-effort cache errors; in findByFileId catch and log
the exception and return Optional.empty(), in save catch and log and skip
setting the cache, and in deleteByFileId catch and log and return without
propagating—refer to methods findByFileId, save, deleteByFileId and the
redisTemplate.opsForValue()/delete calls to locate and update the code.

In `@src/main/java/com/dreamteam/alter/application/file/FileDeleteService.java`:
- Around line 10-12: The FileDeleteService class is missing the Lombok logging
annotation; add `@Slf4j` (import lombok.extern.slf4j.Slf4j) to the class
declaration for structured logging, and update its file deletion methods (e.g.,
any deleteFile, remove, or execute methods inside FileDeleteService) to use
log.info/debug/error calls for start, success, and failure paths including
exception details; ensure the annotation is placed alongside `@Service` and
`@RequiredArgsConstructor` so the generated 'log' field is available.
- Around line 14-15: FileDeleteService currently combines `@Resource` on the
s3Client field with a final field and `@RequiredArgsConstructor`, causing
duplicate injection attempts; fix by choosing one injection style: either remove
the `@Resource` annotation to rely on constructor injection generated by
`@RequiredArgsConstructor` for the final S3Client s3Client field, or remove the
final modifier and keep `@Resource` for field injection; alternatively, if there
are multiple S3Client beans, prefer constructor injection and disambiguate with
`@Qualifier` or mark the intended implementation `@Primary` when using the
constructor-based approach.
- Around line 19-23: FileDeleteService.delete(File file) performs S3 deletion,
state mutation (file.markDeleted()), and cache invalidation directly against
S3Client and PresignedUrlCacheRepository causing partial-failure inconsistencies
and an architecture violation; refactor so FileDeleteService no longer depends
on infrastructure beans directly: introduce an application-level port/adapter
(e.g., FileStoragePort) that abstracts S3 removal and a CacheInvalidationPort
for presigned URL cache, move the transactional boundary into the application
service layer (the callers AdminDeleteFile/ManagerDeleteFile should be
`@Transactional` on the use-case method), and change delete(File) to (1) update DB
state inside the transaction (markDeleted() and persist) and record
intent/event, (2) schedule or invoke the infrastructure ports after commit (or
perform compensating action on failure) so that S3 removal (s3Client) and
presignedUrlCacheRepository operations are executed via the ports and are either
done post-commit or have a clear compensation strategy to avoid DB/S3
divergence.

In `@src/main/java/com/dreamteam/alter/application/file/FileUploadService.java`:
- Around line 15-17: 파일에 로깅 어노테이션이 빠져 있습니다; FileUploadService 클래스 선언에 `@Slf4j`
어노테이션을 추가하여 lombok 기반 로거를 사용하도록 수정하세요 (클래스: FileUploadService, 현재 어노테이션들:
`@Service`, `@RequiredArgsConstructor`). 추가 후 클래스 내부에서 log.* 호출이 가능하도록 import나 다른
변경은 필요 없고 빌드에서 Lombok가 활성화되어 있는지 확인하세요.
- Around line 32-43: Current flow calls s3Client.upload(...) before persisting
the File entity (fileRepository.save(File.create(...))), risking orphaned S3
objects if DB save fails; change to persist the File first with a PENDING (or
UPLOADING) status via File.create(...) and fileRepository.save(...) inside the
transactional boundary, then call s3Client.upload(...) and update the saved
entity to SUCCESS with the returned fileUrl (or mark FAILED and remove the
record on upload failure), or alternately wrap s3Client.upload(...) and
fileRepository.save(...) in a try-catch and on any DB save failure call
s3Client.delete(storedKey, bucketType) to remove the uploaded object; reference
s3Client.upload, fileRepository.save, File.create and the transactional method
when implementing the chosen compensation/update logic.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/AdminDeleteFile.java`:
- Around line 14-16: The AdminDeleteFile class is missing the `@Slf4j` logger
annotation required by the application layer guideline; add the Lombok
annotation (`@Slf4j`) to the AdminDeleteFile class declaration (which implements
AdminDeleteFileUseCase) and ensure the corresponding import
(lombok.extern.slf4j.Slf4j) is present so the generated "log" field can be used
for logging within that class.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/AdminGetPresignedUrl.java`:
- Around line 16-18: Add the missing Lombok logging annotation to the use case
class: annotate the AdminGetPresignedUrl class (which implements
AdminGetPresignedUrlUseCase) with `@Slf4j` so it consistently provides a logger
across application-layer use cases; ensure the import for
lombok.extern.slf4j.Slf4j is present and that the annotation sits above the
class declaration alongside `@Service` and `@RequiredArgsConstructor`.
- Line 7: Remove the unused import of PresignedUrlResult from
AdminGetPresignedUrl: since the result of FileUrlService.getPresignedUrl() is
passed directly into the DTO.of() method and PresignedUrlResult is not
referenced anywhere in this class, delete the import line for PresignedUrlResult
to clean up unused imports.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/AdminUploadFile.java`:
- Around line 21-24: Add a write-operation transaction boundary by annotating
the AdminUploadFile entry point so the whole upload is executed within a single
transaction: apply `@Transactional` to the AdminUploadFile class or the
execute(AdminActor, MultipartFile, FileTargetType, BucketType) method so the
call to fileUploadService.upload(...) and any future pre/post processing in
execute() run inside the same transaction; ensure the chosen placement (class vs
method) matches existing transactional conventions in the codebase.

In `@src/main/java/com/dreamteam/alter/application/file/usecase/AttachFiles.java`:
- Around line 16-18: Add the Lombok logging annotation to the AttachFiles class
by annotating the class with `@Slf4j` (alongside the existing
`@Service`("attachFiles") and `@RequiredArgsConstructor`) so you can use the
generated `log` instance inside methods; ensure the Lombok `@Slf4j` import is
added if missing.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/CleanupOrphanFiles.java`:
- Line 29: The code uses LocalDateTime.now() directly in CleanupOrphanFiles
which makes tests hard to control; inject a java.time.Clock into the class
(e.g., add a Clock field and constructor parameter) and replace
LocalDateTime.now() with LocalDateTime.now(clock) when computing threshold so
tests can supply a fixed Clock; ensure existing constructors are updated or
overloaded and the Clock is used wherever current time is needed in
CleanupOrphanFiles.
- Line 41: Replace the single aggregated log entry in CleanupOrphanFiles that
uses log.info("Orphan file cleanup completed. processed={}", orphanFiles.size())
with a breakdown of success/failure counts: compute the number of successful
deletions and the number of failures from the processing loop that populates
orphanFiles (or from the methods that perform deletion), then log all three
values (processed, succeeded, failed) using log.info and descriptive keys so
operators can monitor outcomes more easily.
- Around line 32-39: The cleanup loop in CleanupOrphanFiles currently calls
s3Client.delete(...) and file.markDeleted() but omits invalidating presigned URL
cache; update the logic to either call the existing
FileDeleteService.delete(file) (which handles s3 deletion, file.markDeleted(),
and cache invalidation) or, if not reusing the service, invoke
presignedUrlCacheRepository.deleteByFileId(file.getId()) after successful
deletion; ensure this is done inside the try block (and only on success) and
keep the existing error logging in the catch so failed deletions do not trigger
cache removal.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/FileCleanupScheduler.java`:
- Around line 10-20: FileCleanupScheduler currently embeds Spring infra
annotations in the application layer; move the scheduling adapter out of
application into the adapter/inbound layer. Remove `@Service`, `@Scheduled`,
`@SchedulerLock` and `@Resource` usage from the FileCleanupScheduler class and leave
only the pure use-case implementation (CleanupOrphanFilesUseCase and its execute
logic) in the application package; then create a new inbound scheduler class
(e.g., a class named FileCleanupScheduler in adapter.inbound) that injects
CleanupOrphanFilesUseCase and applies `@Service/`@Scheduled/@SchedulerLock
annotations and calls cleanupOrphanFiles.execute() from its cleanupOrphanFiles()
method. Ensure the field name cleanupOrphanFiles and method cleanupOrphanFiles()
remain the connectors between the inbound adapter and the application use case.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/ManagerUploadFile.java`:
- Around line 14-16: Add the Lombok logger annotation to the ManagerUploadFile
class: annotate the class ManagerUploadFile with `@Slf4j` (and import
lombok.extern.slf4j.Slf4j) so a logger named "log" is generated; ensure the
Lombok annotation is placed alongside `@Service` and `@RequiredArgsConstructor` and
that the project has Lombok on the classpath/build so the generated logger can
be used in methods of ManagerUploadFile.
- Line 3: The port interface ManagerUploadFileUseCase currently returns the
adapter DTO ManagerUploadFileResponseDto; change the port signature to return a
domain type (e.g., a domain FileUploadResult or primitives/Optional) instead of
ManagerUploadFileResponseDto, update the implementing class ManagerUploadFile to
produce that domain result, and move the conversion from the domain result to
ManagerUploadFileResponseDto into the adapter/controller layer (where the
controller will call ManagerUploadFileUseCase and map the domain result to
ManagerUploadFileResponseDto).

In `@src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java`:
- Around line 33-49: 현재 AwsS3Config의 awsS3Client()와 s3Presigner()에서
accessKey/secretKey를 부분적으로만 설정할 경우 무시하고 기본 credential chain으로 넘어가므로, 꼭 둘 다
설정되었거나 둘 다 비어있어야 하도록 검증을 추가하고 부분 설정이면 애플리케이션 시작 시 예외를 던지세요; 구현 방법: AwsS3Config에
validateAwsCredentials() 같은 헬퍼를 추가해서 StringUtils.hasText(accessKey) XOR
StringUtils.hasText(secretKey)일 경우 IllegalStateException을 던지게 하고, awsS3Client()와
s3Presigner() 시작부에서 해당 검증을 호출한 뒤 두 값이 모두 존재할 때만 StaticCredentialsProvider를 설정하도록
변경하세요.

In `@src/main/java/com/dreamteam/alter/common/util/FileValidator.java`:
- Around line 31-34: The current content-type check in FileValidator (where
file.getContentType() is compared to ALLOWED_CONTENT_TYPES) is insufficient
because it trusts client-provided MIME; update the validator to perform
server-side magic-byte (file signature) validation: in the FileValidator method
handling MultipartFile, read the minimal header bytes from file.getInputStream()
(use mark/reset or copy the header) and match them against known signatures for
allowed types (e.g., JPEG FF D8, PNG 89 50 4E 47, PDF %PDF) in addition to the
existing ALLOWED_CONTENT_TYPES check, and if the signature check fails throw
CustomException(ErrorCode.INVALID_FILE_TYPE); ensure you do not consume the
stream needed later (reset or wrap) and handle IOExceptions by converting them
to the same CustomException.
- Around line 49-54: extractExtension currently returns an empty string for
names like ".hidden" and only the last segment for composites like
"file.tar.gz"; update extractExtension to (1) return "" when fileName is null or
has no dot, (2) treat a leading-dot single-segment name (startsWith(".") and no
other dot) as no extension and return "", and (3) for composite extensions
return the entire suffix starting at the first dot (e.g., ".tar.gz") by using
the first dot index (filename.indexOf('.')) and substring from there; modify the
method extractExtension to implement these checks and return values.

In `@src/main/java/com/dreamteam/alter/domain/file/entity/File.java`:
- Around line 1-26: The File class currently contains infrastructure concerns
(JPA and Spring Data annotations and com.fasterxml.uuid.Generators) that violate
the domain-layer ZERO-dependency rule; refactor by keeping a pure domain model
(retain class name File or rename DomainFile) without any JPA/Spring/third-party
imports or annotations, move all persistence annotations (`@Entity`, `@Table`,
`@Column`, `@Enumerated`, `@CreatedDate`, `@LastModifiedDate`, EntityListeners) into a
new JPA-adapter entity class (e.g., FileJpaEntity) in the outbound/adapter
layer, remove direct use of Generators from the domain (generate IDs in the
adapter or via a domain-friendly ID factory interface), and implement a mapper
(File <-> FileJpaEntity) to translate between domain and persistence
representations (map fields like status, bucketType, targetType, createdAt,
updatedAt).

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminGetPresignedUrlUseCase.java`:
- Around line 3-7: Replace the adapter DTO return type from the domain port
AdminGetPresignedUrlUseCase: change the method signature in
AdminGetPresignedUrlUseCase to return a domain-level result type (e.g.,
PresignedUrlResult) instead of AdminGetPresignedUrlResponseDto, move/define
PresignedUrlResult inside the domain package, and update callers
(controller/adapter) to map PresignedUrlResult ->
AdminGetPresignedUrlResponseDto via a factory method like
AdminGetPresignedUrlResponseDto.of(presignedUrlResult) in the adapter layer;
ensure no imports of AdminGetPresignedUrlResponseDto remain in the domain
package.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminUploadFileUseCase.java`:
- Around line 3-10: The port AdminUploadFileUseCase currently leaks framework
and adapter types—replace the execute signature so it no longer uses
MultipartFile or AdminUploadFileResponseDto; introduce a domain command/input
(e.g., AdminUploadFileCommand containing raw bytes/stream + metadata:
originalFilename, contentType, targetType(FileTargetType),
bucketType(BucketType), and actor id from AdminActor) and return a domain result
type (e.g., AdminUploadFileResult) instead of AdminUploadFileResponseDto; update
the interface method AdminUploadFileUseCase.execute(AdminActor, MultipartFile,
FileTargetType, BucketType) to execute(AdminUploadFileCommand) :
AdminUploadFileResult, and move responsibility for converting MultipartFile →
AdminUploadFileCommand into the controller and mapping AdminUploadFileResult →
AdminUploadFileResponseDto into the adapter/implementation layer.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerGetPresignedUrlUseCase.java`:
- Around line 3-7: The domain port ManagerGetPresignedUrlUseCase currently
depends on the adapter DTO ManagerGetPresignedUrlResponseDto which violates
hexagonal rules; change the use case signature to return a domain type (e.g.,
PresignedUrlResult) instead of ManagerGetPresignedUrlResponseDto, update the
concrete implementation (ManagerGetPresignedUrl) to produce PresignedUrlResult,
and move DTO mapping to the adapter/controller layer so the controller converts
PresignedUrlResult into ManagerGetPresignedUrlResponseDto; ensure no adapter
imports remain in the domain package.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerUploadFileUseCase.java`:
- Around line 3-10: The port interface ManagerUploadFileUseCase currently
depends on MultipartFile and ManagerUploadFileResponseDto; change the contract
so it accepts a domain command (e.g., an UploadFileCommand or similar DTO you
create) instead of org.springframework.web.multipart.MultipartFile and returns a
domain result model (e.g., UploadFileResult) instead of
ManagerUploadFileResponseDto; update the signature in ManagerUploadFileUseCase
(keep ManagerActor, FileTargetType, BucketType as needed or move them into the
new command) and then update the implementing class(es) and controller(s) to
convert the incoming MultipartFile and REST response DTO to/from the new domain
command/result at the adapter layer so the domain port has no Spring or REST
dependencies.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/outbound/FileQueryRepository.java`:
- Around line 10-14: The port interface FileQueryRepository exposes
persistence-style method names; rename the methods to domain-focused contracts:
change findAllByIdIn to findAllByIds and findAllByTargetTypeAndTargetId to
findByTarget (keep findById and findOrphanFiles unchanged), then update all
implementing adapters, tests and call sites to use
FileQueryRepository.findAllByIds(...) and
FileQueryRepository.findByTarget(FileTargetType, String) so the port no longer
leaks Spring Data/JPA naming details.

In `@src/main/java/com/dreamteam/alter/domain/file/port/outbound/S3Client.java`:
- Around line 5-10: S3Client 인터페이스가 인프라 타입 MultipartFile에 의존하고 있으니
upload(MultipartFile file, String storedKey, BucketType bucketType) 시그니처를 도메인 소유
모델(예: UploadSource with InputStream, contentLength, contentType,
originalFilename)로 교체하고 인터페이스( S3Client )와 관련 구현체 모두에서 해당 타입으로 변경하세요; 웹
레이어/어댑터에서 MultipartFile을 UploadSource로 변환하여 S3Client.upload를 호출하도록 이동하고,
getPresignedUrl(String storedKey, BucketType bucketType)와 delete(...) 메서드는 그대로
두되 관련 import와 구현을 함께 리팩터링하고 유닛/통합 테스트를 업데이트하세요.

In `@src/main/resources/application.yml`:
- Around line 10-13: 설정된 servlet.multipart.max-file-size와
servlet.multipart.max-request-size가 둘 다 20MB로 동일해 multipart 헤더/경계 오버헤드로 인해 정상
파일이 413 에러가 발생할 수 있으니 servlet.multipart.max-request-size를 max-file-size보다 충분히
크게(예: 20MB 파일이면 25MB 이상 또는 파일 크기 + 여유율)로 늘리도록 application.yml의
servlet.multipart.max-request-size 값을 조정하세요.

---

Outside diff comments:
In `@src/main/resources/application.yml`:
- Around line 75-83: The YAML placeholders for AWS credentials should include
empty defaults so Spring doesn't fail when env vars are absent; update the
application.yml entries that use ${AWS_ACCESS_KEY_ID} and
${AWS_SECRET_ACCESS_KEY} to include default empty values (e.g.
${AWS_ACCESS_KEY_ID:} and ${AWS_SECRET_ACCESS_KEY:}) so AwsS3Config and
AwsSesConfig (which rely on
`@Value`("${aws.access-key:}")/@Value("${aws.secret-key:}")) can fall back to the
AWS SDK default credential chain instead of preventing app startup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 69bfa77b-7b3e-409c-aa41-4a55fad75eb7

📥 Commits

Reviewing files that changed from the base of the PR and between c22975e and 84fb194.

📒 Files selected for processing (50)
  • build.gradle
  • src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileController.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileControllerSpec.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/dto/AdminGetPresignedUrlResponseDto.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/dto/AdminUploadFileResponseDto.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/common/dto/FileResponseDto.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileController.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileControllerSpec.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/dto/ManagerGetPresignedUrlResponseDto.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/dto/ManagerUploadFileResponseDto.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/persistence/FileJpaRepository.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/persistence/FileQueryRepositoryImpl.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/persistence/FileRepositoryImpl.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/redis/PresignedUrlCacheEntry.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/redis/PresignedUrlCacheRepositoryImpl.java
  • src/main/java/com/dreamteam/alter/application/file/FileDeleteService.java
  • src/main/java/com/dreamteam/alter/application/file/FileUploadService.java
  • src/main/java/com/dreamteam/alter/application/file/FileUrlService.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/AdminDeleteFile.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/AdminGetPresignedUrl.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/AdminUploadFile.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/AttachFiles.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/CleanupOrphanFiles.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/FileCleanupScheduler.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/ManagerDeleteFile.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/ManagerGetPresignedUrl.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/ManagerUploadFile.java
  • src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java
  • src/main/java/com/dreamteam/alter/common/exception/ErrorCode.java
  • src/main/java/com/dreamteam/alter/common/util/FileValidator.java
  • src/main/java/com/dreamteam/alter/domain/file/PresignedUrlResult.java
  • src/main/java/com/dreamteam/alter/domain/file/entity/File.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminDeleteFileUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminGetPresignedUrlUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminUploadFileUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/AttachFilesUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/CleanupOrphanFilesUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerDeleteFileUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerGetPresignedUrlUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerUploadFileUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/outbound/FileQueryRepository.java
  • src/main/java/com/dreamteam/alter/domain/file/port/outbound/FileRepository.java
  • src/main/java/com/dreamteam/alter/domain/file/port/outbound/PresignedUrlCacheRepository.java
  • src/main/java/com/dreamteam/alter/domain/file/port/outbound/S3Client.java
  • src/main/java/com/dreamteam/alter/domain/file/type/BucketType.java
  • src/main/java/com/dreamteam/alter/domain/file/type/FileStatus.java
  • src/main/java/com/dreamteam/alter/domain/file/type/FileTargetType.java
  • src/main/resources/application.yml
  • src/test/resources/application.yml

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (9)
src/main/java/com/dreamteam/alter/application/file/usecase/CleanupOrphanFiles.java (1)

29-29: 🧹 Nitpick | 🔵 Trivial

테스트 용이성: Clock 주입 고려

LocalDateTime.now()를 직접 호출하면 단위 테스트에서 시간 제어가 어렵습니다. 이전 리뷰에서 언급된 사항으로, java.time.Clock을 주입받아 사용하면 테스트 시 시간을 고정할 수 있습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/CleanupOrphanFiles.java`
at line 29, Replace the direct call to LocalDateTime.now() in CleanupOrphanFiles
with a Clock-backed call to improve testability: add a java.time.Clock field to
the CleanupOrphanFiles class (inject via constructor), and change the threshold
computation from LocalDateTime.now().minusHours(24) to
LocalDateTime.now(clock).minusHours(24); ensure constructors or DI provide
Clock.systemDefaultZone() (or a default) for production and allow tests to pass
a fixed Clock.
src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java (1)

77-79: ⚠️ Potential issue | 🟠 Major

Presign 실패를 업로드 실패 코드로 매핑하지 마세요.

Line 79에서 FILE_UPLOAD_FAILED를 던지면 presigned URL 조회 실패가 업로드 실패로 집계되고, 클라이언트도 잘못된 실패 사유를 받습니다. presign 전용 에러 코드를 쓰거나 최소한 업로드와 구분되는 코드로 분리하세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java`
around lines 77 - 79, The catch in S3ClientImpl that currently logs and throws
new CustomException(ErrorCode.FILE_UPLOAD_FAILED) for presign failures must be
changed to use a dedicated presign error code (e.g.,
ErrorCode.FILE_PRESIGN_FAILED or PRESIGN_FAILED) so presign errors aren't
misclassified as upload failures; update the throw in the presign method (the
block catching Exception e where it logs "S3 presign failed for key={}") to
throw the new presign-specific CustomException and adjust the log message if
needed to keep context.
src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerGetPresignedUrlUseCase.java (1)

3-7: ⚠️ Potential issue | 🟠 Major

도메인 포트에서 어댑터 DTO 의존을 제거하세요.

Line 3의 FilePresignedUrlResponseDto import와 Line 7의 반환 타입 때문에 domain 포트가 inbound adapter에 직접 의존합니다. 이 포트는 PresignedUrlResult 같은 도메인 타입을 반환하고, DTO 변환은 컨트롤러에서 처리해야 합니다. As per coding guidelines, "ZERO infrastructure dependencies (no Spring, no JPA annotations, no external libs)." and "Port interfaces (inbound/outbound) are clean contracts with no implementation details."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerGetPresignedUrlUseCase.java`
around lines 3 - 7, The domain inbound port ManagerGetPresignedUrlUseCase
currently depends on an adapter DTO (FilePresignedUrlResponseDto) via the
execute method return type; change the port to return a domain-specific result
(e.g., PresignedUrlResult) instead and move DTO mapping into the
controller/adapter layer. Specifically, update the
ManagerGetPresignedUrlUseCase.execute(ManagerActor actor, String fileId)
signature to return PresignedUrlResult (a domain type you create if missing),
remove the import of FilePresignedUrlResponseDto from the port, and perform
conversion from PresignedUrlResult to FilePresignedUrlResponseDto inside the
inbound adapter/controller that implements the interface.
src/main/java/com/dreamteam/alter/application/file/usecase/ManagerUploadFile.java (1)

3-5: ⚠️ Potential issue | 🟠 Major

유스케이스가 어댑터 DTO를 직접 반환하지 않게 분리하세요.

Line 24의 반환 타입과 Line 26의 DTO 생성 때문에 application 레이어가 adapter.inbound에 역의존합니다. 유스케이스는 fileId 같은 도메인/애플리케이션 결과만 반환하고, ManagerUploadFileResponseDto.of(...) 변환은 컨트롤러로 옮기는 편이 계층 경계를 지킵니다. As per coding guidelines, "Port interfaces (inbound/outbound) are clean contracts with no implementation details."

Also applies to: 24-26

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/ManagerUploadFile.java`
around lines 3 - 5, The ManagerUploadFile use-case implementation currently
returns an adapter DTO and creates ManagerUploadFileResponseDto via
ManagerUploadFileResponseDto.of(...), causing an application->adapter inbound
dependency; change the ManagerUploadFile.upload(...) implementation (and the
ManagerUploadFileUseCase method signature) to return a domain/application
primitive (e.g. String fileId or a simple FileUploadResult domain type) instead
of ManagerUploadFileResponseDto, have upload call FileUploadService.upload(...)
and return only the resulting fileId, and move the
ManagerUploadFileResponseDto.of(...) conversion into the controller/adapter so
the application layer no longer imports or constructs
ManagerUploadFileResponseDto.
src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminGetPresignedUrlUseCase.java (1)

3-7: ⚠️ Potential issue | 🟠 Major

도메인 포트에서 어댑터 DTO 의존을 제거하세요.

Line 3의 FilePresignedUrlResponseDto import와 Line 7의 반환 타입 때문에 domain 포트가 inbound adapter에 직접 의존합니다. 이 포트는 PresignedUrlResult 같은 도메인 타입을 반환하고, DTO 변환은 컨트롤러에서 처리해야 합니다. As per coding guidelines, "ZERO infrastructure dependencies (no Spring, no JPA annotations, no external libs)." and "Port interfaces (inbound/outbound) are clean contracts with no implementation details."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminGetPresignedUrlUseCase.java`
around lines 3 - 7, The AdminGetPresignedUrlUseCase domain port currently
depends on the adapter DTO FilePresignedUrlResponseDto; change the execute
signature to return a pure domain type (e.g., PresignedUrlResult) instead of
FilePresignedUrlResponseDto, keep the AdminActor parameter, and remove the
import of FilePresignedUrlResponseDto; perform DTO mapping in the inbound
adapter/controller layer where FilePresignedUrlResponseDto is available so the
domain port (interface AdminGetPresignedUrlUseCase and its execute method) has
no infrastructure or adapter dependencies.
src/main/java/com/dreamteam/alter/application/file/usecase/AdminGetPresignedUrl.java (1)

15-17: 🧹 Nitpick | 🔵 Trivial

@Slf4j 어노테이션을 추가해 주세요.

코딩 가이드라인에 따르면 Application 레이어의 Use Case는 @Slf4j를 사용해야 합니다. 현재 로깅이 필요하지 않더라도 일관성을 위해 추가를 권장합니다.

As per coding guidelines: "Logging uses @Slf4j."

📝 제안된 수정
 import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
+@Slf4j
 `@Service`("adminGetPresignedUrl")
 `@RequiredArgsConstructor`
 public class AdminGetPresignedUrl implements AdminGetPresignedUrlUseCase {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/AdminGetPresignedUrl.java`
around lines 15 - 17, The class AdminGetPresignedUrl is missing the Lombok
logging annotation; add the `@Slf4j` annotation (import lombok.extern.slf4j.Slf4j)
to the AdminGetPresignedUrl class alongside `@Service` and
`@RequiredArgsConstructor` so the generated 'log' logger is available for
consistent Application-layer logging (e.g., use 'log' in methods of
AdminGetPresignedUrl when needed).
src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileController.java (1)

21-25: 🧹 Nitpick | 🔵 Trivial

컨트롤러에 @Getter 어노테이션을 추가해 주세요.

코딩 가이드라인에 따르면 컨트롤러는 @Getter, @RequiredArgsConstructor를 사용해야 합니다. 현재 @Getter가 누락되어 있습니다.

As per coding guidelines: "Controllers use @Getter, @RequiredArgsConstructor (no @Setter)."

♻️ 제안된 수정
+import lombok.Getter;
 import lombok.RequiredArgsConstructor;

 `@RestController`
 `@RequestMapping`("/admin/files")
 `@PreAuthorize`("hasAnyRole('ADMIN')")
+@Getter
 `@RequiredArgsConstructor`
 public class AdminFileController implements AdminFileControllerSpec {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileController.java`
around lines 21 - 25, Add the missing Lombok `@Getter` annotation to the
AdminFileController class declaration so it uses both `@Getter` and
`@RequiredArgsConstructor` as per the coding guideline; update the class
declaration for AdminFileController (public class AdminFileController implements
AdminFileControllerSpec) to include `@Getter` above it alongside the existing
`@RestController`, `@RequestMapping`, `@PreAuthorize`, and `@RequiredArgsConstructor`
annotations.
src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileController.java (2)

27-34: 🧹 Nitpick | 🔵 Trivial

@Resource@RequiredArgsConstructor 조합을 명확히 해주세요.

@Resource는 필드 주입용 애노테이션이고 @RequiredArgsConstructor는 생성자 주입용입니다. 현재는 인터페이스 구현이 하나씩이라 작동하지만, 구현체가 추가될 경우 예상치 못한 빈 선택 문제가 발생할 수 있습니다. 명시적 생성자 + @Qualifier 패턴을 권장합니다.

♻️ 제안된 수정
-import jakarta.annotation.Resource;
-import lombok.RequiredArgsConstructor;
+import lombok.Getter;
+import org.springframework.beans.factory.annotation.Qualifier;

 `@RestController`
 `@RequestMapping`("/manager/files")
 `@PreAuthorize`("hasAnyRole('MANAGER')")
-@RequiredArgsConstructor
+@Getter
 public class ManagerFileController implements ManagerFileControllerSpec {

-    `@Resource`(name = "managerUploadFile")
     private final ManagerUploadFileUseCase managerUploadFile;
-
-    `@Resource`(name = "managerGetPresignedUrl")
     private final ManagerGetPresignedUrlUseCase managerGetPresignedUrl;
-
-    `@Resource`(name = "managerDeleteFile")
     private final ManagerDeleteFileUseCase managerDeleteFile;
+
+    public ManagerFileController(
+        `@Qualifier`("managerUploadFile") ManagerUploadFileUseCase managerUploadFile,
+        `@Qualifier`("managerGetPresignedUrl") ManagerGetPresignedUrlUseCase managerGetPresignedUrl,
+        `@Qualifier`("managerDeleteFile") ManagerDeleteFileUseCase managerDeleteFile
+    ) {
+        this.managerUploadFile = managerUploadFile;
+        this.managerGetPresignedUrl = managerGetPresignedUrl;
+        this.managerDeleteFile = managerDeleteFile;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileController.java`
around lines 27 - 34, The fields managerUploadFile, managerGetPresignedUrl, and
managerDeleteFile in ManagerFileController currently use `@Resource` (field
injection) alongside `@RequiredArgsConstructor`; replace this ambiguous
combination with an explicit constructor that injects these three dependencies
and annotate each constructor parameter with `@Qualifier` specifying the bean name
(e.g., "managerUploadFile", "managerGetPresignedUrl", "managerDeleteFile");
remove the `@Resource` annotations and `@RequiredArgsConstructor` so the controller
uses constructor injection and avoids ambiguous bean selection when multiple
implementations exist.

21-25: 🧹 Nitpick | 🔵 Trivial

컨트롤러에 @Getter 어노테이션을 추가해 주세요.

코딩 가이드라인에 따르면 컨트롤러는 @Getter, @RequiredArgsConstructor를 사용해야 합니다. 현재 @Getter가 누락되어 있습니다.

As per coding guidelines: "Controllers use @Getter, @RequiredArgsConstructor (no @Setter)."

♻️ 제안된 수정
+import lombok.Getter;
 import lombok.RequiredArgsConstructor;
 
 `@RestController`
 `@RequestMapping`("/manager/files")
 `@PreAuthorize`("hasAnyRole('MANAGER')")
+@Getter
 `@RequiredArgsConstructor`
 public class ManagerFileController implements ManagerFileControllerSpec {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileController.java`
around lines 21 - 25, Add the missing Lombok `@Getter` annotation to the
ManagerFileController class declaration (which currently has `@RestController`,
`@RequestMapping`("/manager/files"), `@PreAuthorize`("hasAnyRole('MANAGER')"), and
`@RequiredArgsConstructor`) so the controller follows the project guideline of
using `@Getter` and `@RequiredArgsConstructor` (ensure you do not add any `@Setter`).
This change should be made on the ManagerFileController class to expose required
final fields via generated getters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@build.gradle`:
- Around line 50-52: Replace per-module AWS SDK versions with the AWS SDK BOM:
add a platform dependency for "software.amazon.awssdk:bom" (pinned to the
desired BOM version) in the dependencies block and remove the version suffixes
from "software.amazon.awssdk:ses" and "software.amazon.awssdk:s3" so they
inherit versions from the BOM; leave
"com.fasterxml.uuid:java-uuid-generator:5.1.0" as an explicit versioned
dependency. Ensure you reference the BOM via platform(...) (or
implementation(platform(...)) depending on Gradle DSL) and verify the dependency
names "software.amazon.awssdk:ses" and "software.amazon.awssdk:s3" are declared
without versions.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/file/scheduler/FileCleanupScheduler.java`:
- Around line 14-15: 현재 FileCleanupScheduler 클래스에서 필드 cleanupOrphanFiles에
`@Resource와` `@RequiredArgsConstructor가` 중복되어 있으니 `@Resource를` 제거하고 생성자 주입 방식에 맞춰
`@Qualifier를` 사용하도록 변경하세요; 구체적으로 FileCleanupScheduler의 private final
CleanupOrphanFilesUseCase cleanupOrphanFiles 필드에 붙어있는 `@Resource` 어노테이션을 삭제하고, 클래스
레벨의 `@RequiredArgsConstructor를` 유지한 채 생성자 주입 시 대상 빈을 지정하기 위해
`@Qualifier`("cleanupOrphanFiles")를 해당 생성자 파라미터 또는 필드(롬복 사용 시 `@Qualifier를` 필드에 선언)로
적용하여 CleanupOrphanFilesUseCase가 올바른 빈을 주입받도록 수정하세요.
- Around line 17-21: 파일 스케줄러의 cleanupOrphanFiles()가 cleanupOrphanFiles.execute()
호출 중 예외가 발생하면 조용히 실패하므로 호출을 try-catch로 감싸고 예외를 로깅하도록 수정하세요: FileCleanupScheduler
클래스의 cleanupOrphanFiles() 메서드에서 cleanupOrphanFiles.execute()를 try { ... } catch
(Exception e) { logger.error("Failed to execute cleanupOrphanFiles", e); } 형태로
예외를 잡고 Logger(예: LoggerFactory.getLogger(FileCleanupScheduler.class))를 사용해 명확한
컨텍스트 메시지와 스택트레이스를 남기도록 구현하세요.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/AdminUploadFile.java`:
- Around line 3-5: The AdminUploadFile use case currently returns an adapter DTO
(AdminUploadFileResponseDto via AdminUploadFileResponseDto.of(...)), coupling
application layer to HTTP model; change the AdminUploadFile.execute (or relevant
method) to return only the application/domain result (e.g., a fileId or a small
value object) instead of AdminUploadFileResponseDto, update any callers to
accept that domain result, and move the DTO construction
(AdminUploadFileResponseDto.of(...)) into the controller/adaptor layer that
handles HTTP responses; adjust FileUploadService interaction and method
signatures accordingly so the application layer depends only on domain types.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/CleanupOrphanFiles.java`:
- Around line 23-24: The class CleanupOrphanFiles currently mixes `@Resource` on
the final field fileDeleteService with Lombok's `@RequiredArgsConstructor`; remove
the `@Resource` annotation and rely on constructor injection generated by
`@RequiredArgsConstructor`, adding `@Qualifier`("fileDeleteService") to the
injection point so the correct bean is injected (import
org.springframework.beans.factory.annotation.Qualifier as needed); specifically,
remove `@Resource` from the fileDeleteService field and annotate the
constructor-injected parameter (or the field) with
`@Qualifier`("fileDeleteService") to keep consistent constructor-based injection.

In
`@src/main/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandler.java`:
- Around line 70-82: The handlers handleMissingServletRequestPartException and
handleMissingServletRequestParameterException currently return generic messages
and ignore the exception details; update each to extract the missing name from
the exception via e.getRequestPartName() for MissingServletRequestPartException
and e.getParameterName() for MissingServletRequestParameterException and include
that name in the ErrorResponse message (e.g., "Multipart '{name}'가 누락됐습니다." /
"파라미터 '{name}'가 누락됐습니다."), preserving the same ErrorCode and status behavior.

---

Duplicate comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileController.java`:
- Around line 21-25: Add the missing Lombok `@Getter` annotation to the
AdminFileController class declaration so it uses both `@Getter` and
`@RequiredArgsConstructor` as per the coding guideline; update the class
declaration for AdminFileController (public class AdminFileController implements
AdminFileControllerSpec) to include `@Getter` above it alongside the existing
`@RestController`, `@RequestMapping`, `@PreAuthorize`, and `@RequiredArgsConstructor`
annotations.

In
`@src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileController.java`:
- Around line 27-34: The fields managerUploadFile, managerGetPresignedUrl, and
managerDeleteFile in ManagerFileController currently use `@Resource` (field
injection) alongside `@RequiredArgsConstructor`; replace this ambiguous
combination with an explicit constructor that injects these three dependencies
and annotate each constructor parameter with `@Qualifier` specifying the bean name
(e.g., "managerUploadFile", "managerGetPresignedUrl", "managerDeleteFile");
remove the `@Resource` annotations and `@RequiredArgsConstructor` so the controller
uses constructor injection and avoids ambiguous bean selection when multiple
implementations exist.
- Around line 21-25: Add the missing Lombok `@Getter` annotation to the
ManagerFileController class declaration (which currently has `@RestController`,
`@RequestMapping`("/manager/files"), `@PreAuthorize`("hasAnyRole('MANAGER')"), and
`@RequiredArgsConstructor`) so the controller follows the project guideline of
using `@Getter` and `@RequiredArgsConstructor` (ensure you do not add any `@Setter`).
This change should be made on the ManagerFileController class to expose required
final fields via generated getters.

In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java`:
- Around line 77-79: The catch in S3ClientImpl that currently logs and throws
new CustomException(ErrorCode.FILE_UPLOAD_FAILED) for presign failures must be
changed to use a dedicated presign error code (e.g.,
ErrorCode.FILE_PRESIGN_FAILED or PRESIGN_FAILED) so presign errors aren't
misclassified as upload failures; update the throw in the presign method (the
block catching Exception e where it logs "S3 presign failed for key={}") to
throw the new presign-specific CustomException and adjust the log message if
needed to keep context.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/AdminGetPresignedUrl.java`:
- Around line 15-17: The class AdminGetPresignedUrl is missing the Lombok
logging annotation; add the `@Slf4j` annotation (import lombok.extern.slf4j.Slf4j)
to the AdminGetPresignedUrl class alongside `@Service` and
`@RequiredArgsConstructor` so the generated 'log' logger is available for
consistent Application-layer logging (e.g., use 'log' in methods of
AdminGetPresignedUrl when needed).

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/CleanupOrphanFiles.java`:
- Line 29: Replace the direct call to LocalDateTime.now() in CleanupOrphanFiles
with a Clock-backed call to improve testability: add a java.time.Clock field to
the CleanupOrphanFiles class (inject via constructor), and change the threshold
computation from LocalDateTime.now().minusHours(24) to
LocalDateTime.now(clock).minusHours(24); ensure constructors or DI provide
Clock.systemDefaultZone() (or a default) for production and allow tests to pass
a fixed Clock.

In
`@src/main/java/com/dreamteam/alter/application/file/usecase/ManagerUploadFile.java`:
- Around line 3-5: The ManagerUploadFile use-case implementation currently
returns an adapter DTO and creates ManagerUploadFileResponseDto via
ManagerUploadFileResponseDto.of(...), causing an application->adapter inbound
dependency; change the ManagerUploadFile.upload(...) implementation (and the
ManagerUploadFileUseCase method signature) to return a domain/application
primitive (e.g. String fileId or a simple FileUploadResult domain type) instead
of ManagerUploadFileResponseDto, have upload call FileUploadService.upload(...)
and return only the resulting fileId, and move the
ManagerUploadFileResponseDto.of(...) conversion into the controller/adapter so
the application layer no longer imports or constructs
ManagerUploadFileResponseDto.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminGetPresignedUrlUseCase.java`:
- Around line 3-7: The AdminGetPresignedUrlUseCase domain port currently depends
on the adapter DTO FilePresignedUrlResponseDto; change the execute signature to
return a pure domain type (e.g., PresignedUrlResult) instead of
FilePresignedUrlResponseDto, keep the AdminActor parameter, and remove the
import of FilePresignedUrlResponseDto; perform DTO mapping in the inbound
adapter/controller layer where FilePresignedUrlResponseDto is available so the
domain port (interface AdminGetPresignedUrlUseCase and its execute method) has
no infrastructure or adapter dependencies.

In
`@src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerGetPresignedUrlUseCase.java`:
- Around line 3-7: The domain inbound port ManagerGetPresignedUrlUseCase
currently depends on an adapter DTO (FilePresignedUrlResponseDto) via the
execute method return type; change the port to return a domain-specific result
(e.g., PresignedUrlResult) instead and move DTO mapping into the
controller/adapter layer. Specifically, update the
ManagerGetPresignedUrlUseCase.execute(ManagerActor actor, String fileId)
signature to return PresignedUrlResult (a domain type you create if missing),
remove the import of FilePresignedUrlResponseDto from the port, and perform
conversion from PresignedUrlResult to FilePresignedUrlResponseDto inside the
inbound adapter/controller that implements the interface.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8c9fcaa4-2a98-4f7c-9be2-568effc631f6

📥 Commits

Reviewing files that changed from the base of the PR and between 84fb194 and d3260f9.

📒 Files selected for processing (18)
  • build.gradle
  • src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileController.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/admin/file/controller/AdminFileControllerSpec.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/common/dto/FilePresignedUrlResponseDto.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/general/file/scheduler/FileCleanupScheduler.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileController.java
  • src/main/java/com/dreamteam/alter/adapter/inbound/manager/file/controller/ManagerFileControllerSpec.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/redis/PresignedUrlCacheEntry.java
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/redis/PresignedUrlCacheRepositoryImpl.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/AdminGetPresignedUrl.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/AdminUploadFile.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/CleanupOrphanFiles.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/ManagerGetPresignedUrl.java
  • src/main/java/com/dreamteam/alter/application/file/usecase/ManagerUploadFile.java
  • src/main/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandler.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/AdminGetPresignedUrlUseCase.java
  • src/main/java/com/dreamteam/alter/domain/file/port/inbound/ManagerGetPresignedUrlUseCase.java

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (3)
src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java (2)

27-27: 🧹 Nitpick | 🔵 Trivial

Public URL 형식을 설정값으로 분리 고려

PUBLIC_URL_FORMAT이 하드코딩되어 있습니다. 버킷 리전이나 커스텀 도메인에 따라 URL 형식이 달라질 수 있으므로, @Value로 설정값을 분리하면 유연성이 높아집니다.

설정 분리 예시
-    private static final String PUBLIC_URL_FORMAT = "https://%s.s3.%s.amazonaws.com/%s";
+    `@Value`("${aws.s3.public-url-format:https://%s.s3.%s.amazonaws.com/%s}")
+    private String publicUrlFormat;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java`
at line 27, PUBLIC_URL_FORMAT is hardcoded in S3ClientImpl; change it to a
configurable property injected via `@Value` (e.g., add a private final String
publicUrlFormat field and populate it from a property like
"${s3.publicUrlFormat: https://%s.s3.%s.amazonaws.com/%s}" via constructor or
field injection) so environments with different regions or custom domains can
override the format; update usages of PUBLIC_URL_FORMAT in S3ClientImpl to use
the injected publicUrlFormat and keep a sensible default to avoid breaking
existing deployments.

82-85: 🛠️ Refactor suggestion | 🟠 Major

Presign 실패 시 전용 에러 코드 사용 권장

getPresignedUrl() 실패 시 INTERNAL_SERVER_ERROR를 사용하고 있습니다. upload()FILE_UPLOAD_FAILED, delete()FILE_DELETE_FAILED를 사용하는 것처럼, presign 전용 에러 코드(예: FILE_PRESIGN_FAILED)를 추가하면 클라이언트에서 오류 원인을 더 명확히 파악할 수 있습니다.

에러 코드 추가 예시
 // ErrorCode.java에 추가
+FILE_PRESIGN_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, "F004", "파일 URL 생성에 실패했습니다."),

 // S3ClientImpl.java
 } catch (Exception e) {
     log.error("S3 presign failed for key={}", storedKey, e);
-    throw new CustomException(ErrorCode.INTERNAL_SERVER_ERROR);
+    throw new CustomException(ErrorCode.FILE_PRESIGN_FAILED);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java`
around lines 82 - 85, The catch block in S3ClientImpl.getPresignedUrl currently
throws a generic INTERNAL_SERVER_ERROR; add a specific error code (e.g.,
FILE_PRESIGN_FAILED) to the ErrorCode enum and update the catch to throw new
CustomException(ErrorCode.FILE_PRESIGN_FAILED) while keeping the existing log
call; mirror the pattern used by upload() and delete() so clients can
distinguish presign failures from other S3 errors.
src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java (1)

44-47: ⚠️ Potential issue | 🟠 Major

정적 자격증명 부분 설정 시 검증 누락

accessKeysecretKey 중 하나만 설정된 경우, 조용히 기본 credential chain으로 넘어갑니다. 설정 오류로 인해 의도치 않은 AWS 계정에 접근할 수 있으므로, 부분 설정 시 애플리케이션 시작을 실패시키는 것이 안전합니다.

검증 메서드 추가 제안
+    private void validateStaticCredentials() {
+        if (StringUtils.hasText(accessKey) != StringUtils.hasText(secretKey)) {
+            throw new IllegalStateException(
+                "aws.access-key and aws.secret-key must be configured together"
+            );
+        }
+    }
+
     `@Bean`
     public S3Client awsS3Client() {
+        validateStaticCredentials();
         // ... existing code
     }
 
     `@Bean`
     public S3Presigner s3Presigner() {
+        validateStaticCredentials();
         // ... existing code
     }

Also applies to: 57-60

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java` around
lines 44 - 47, The code in AwsS3Config silently falls back to the default
credential chain when only one of accessKey/secretKey is provided; add a
validation that throws an exception (e.g., IllegalStateException) during
configuration startup if exactly one of accessKey or secretKey is set so the app
fails fast; implement this check in AwsS3Config (create a private
validateStaticCredentials or validateAwsCredentials method) and call it before
the AwsBasicCredentials.create(...) block (and duplicate the same validation for
the other credential block referenced at lines 57-60) using
StringUtils.hasText(accessKey)/hasText(secretKey) to detect partial
configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java`:
- Around line 52-63: The s3Presigner() method lacks the same
ClientOverrideConfiguration timeouts used by awsS3Client(), causing
inconsistency; update s3Presigner() (S3Presigner.builder()/builder) to apply the
existing ClientOverrideConfiguration (the one created for awsS3Client()) via the
builder.overrideConfiguration(...) or equivalent setter and ensure the same
timeout/retry settings are reused (find the ClientOverrideConfiguration creation
and apply it to the S3Presigner.Builder before builder.build()).

In `@src/main/java/com/dreamteam/alter/common/util/FileValidator.java`:
- Around line 49-54: The extractExtension method returns "." when the filename
ends with a dot (e.g., "file.") because it always prepends '.' to the substring
even if the substring is empty; update FileValidator.extractExtension to detect
if the last '.' is the final character (fileName.lastIndexOf('.') ==
fileName.length() - 1) and return an empty string in that case, otherwise keep
the existing behavior (return "." + substring(...).toLowerCase()); ensure this
handles single-dot filenames and trailing-dot cases.
- Line 12: FileValidator is a utility class with only static methods and should
prevent instantiation; add a private no-arg constructor to the FileValidator
class (e.g., private FileValidator()) to block external instantiation and
optionally throw an AssertionError or IllegalStateException inside the
constructor to guard against reflection-based construction.

---

Duplicate comments:
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java`:
- Line 27: PUBLIC_URL_FORMAT is hardcoded in S3ClientImpl; change it to a
configurable property injected via `@Value` (e.g., add a private final String
publicUrlFormat field and populate it from a property like
"${s3.publicUrlFormat: https://%s.s3.%s.amazonaws.com/%s}" via constructor or
field injection) so environments with different regions or custom domains can
override the format; update usages of PUBLIC_URL_FORMAT in S3ClientImpl to use
the injected publicUrlFormat and keep a sensible default to avoid breaking
existing deployments.
- Around line 82-85: The catch block in S3ClientImpl.getPresignedUrl currently
throws a generic INTERNAL_SERVER_ERROR; add a specific error code (e.g.,
FILE_PRESIGN_FAILED) to the ErrorCode enum and update the catch to throw new
CustomException(ErrorCode.FILE_PRESIGN_FAILED) while keeping the existing log
call; mirror the pattern used by upload() and delete() so clients can
distinguish presign failures from other S3 errors.

In `@src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java`:
- Around line 44-47: The code in AwsS3Config silently falls back to the default
credential chain when only one of accessKey/secretKey is provided; add a
validation that throws an exception (e.g., IllegalStateException) during
configuration startup if exactly one of accessKey or secretKey is set so the app
fails fast; implement this check in AwsS3Config (create a private
validateStaticCredentials or validateAwsCredentials method) and call it before
the AwsBasicCredentials.create(...) block (and duplicate the same validation for
the other credential block referenced at lines 57-60) using
StringUtils.hasText(accessKey)/hasText(secretKey) to detect partial
configuration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dc4b04bb-f785-41db-864c-0da96c54dbda

📥 Commits

Reviewing files that changed from the base of the PR and between d3260f9 and d34de42.

📒 Files selected for processing (3)
  • src/main/java/com/dreamteam/alter/adapter/outbound/file/external/S3ClientImpl.java
  • src/main/java/com/dreamteam/alter/common/config/AwsS3Config.java
  • src/main/java/com/dreamteam/alter/common/util/FileValidator.java

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/com/dreamteam/alter/common/util/FileValidator.java`:
- Around line 38-42: sanitizeFileName currently uses
Paths.get(fileName).getFileName() which only strips separators for the host OS;
instead first handle null (keep existing null check), then normalize backslashes
to forward slashes (replace '\\' with '/'), trim whitespace, split on '/' and
take the last non-empty segment as the basename, and if that result is empty
return "unknown"; update the sanitizeFileName method (and remove reliance on
Paths.get(...).getFileName()) to perform this normalization and fallback logic.
- Around line 45-56: Both buildStoredKey(...) and extractExtension(...) call
String#toLowerCase() without a locale, which can produce incorrect results under
certain JVM locales (e.g., Turkish); update both to use toLowerCase(Locale.ROOT)
to ensure locale-independent lowercasing, and add the necessary java.util.Locale
import if missing so buildStoredKey(targetType, extension) and
extractExtension(fileName) always produce consistent, locale-neutral results for
S3 keys and extensions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8784d046-88d2-49d0-87d2-8c672f79234a

📥 Commits

Reviewing files that changed from the base of the PR and between d34de42 and 9843952.

📒 Files selected for processing (1)
  • src/main/java/com/dreamteam/alter/common/util/FileValidator.java

@ysw789 ysw789 merged commit 23e9e35 into dev Mar 9, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants