[REFACTOR] Presigned URL API 공용으로 분리#42
Conversation
|
Warning Review limit reached
More reviews will be available in 35 minutes and 45 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough도메인별( ChangesS3 Presigned URL 공용화 리팩토링
Sequence Diagram(s)sequenceDiagram
participant Client
participant S3FileController
participant S3FileService
participant S3UploadManager
rect rgba(70, 130, 180, 0.5)
Note over Client,S3UploadManager: Presigned URL 발급 흐름
Client->>S3FileController: POST /api/v1/files/presigned-url (PresignRequest)
S3FileController->>S3FileService: issuePresignedUpload(target, contentType)
S3FileService->>S3FileService: S3ContentType 변환 및 UploadTarget.allowed 검증
alt 허용되지 않는 contentType
S3FileService-->>S3FileController: GeneralException(UNSUPPORTED_CONTENT_TYPE)
S3FileController-->>Client: 400 에러 응답
else 허용됨
S3FileService->>S3UploadManager: issueTempUpload(dir, contentType)
S3UploadManager-->>S3FileService: PresignedUpload(key, presignedUrl)
S3FileService-->>S3FileController: PresignedUpload
S3FileController-->>Client: ApiResponse(PresignResponse)
end
end
rect rgba(60, 179, 113, 0.5)
Note over Client,S3UploadManager: 업로드 키 검증 흐름 (PortfolioService / CommissionCreateFileService)
Client->>S3FileService: validateUploadedKeys(target, keys)
S3FileService->>S3FileService: null / 중복 키 검사
loop 각 key
S3FileService->>S3UploadManager: isTempKey(key)
S3FileService->>S3UploadManager: getObjectSize(key)
alt 객체 없음 또는 크기 초과
S3FileService-->>Client: GeneralException(INVALID_FILE / FILE_SIZE_EXCEEDED)
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/main/java/ditda/backend/global/s3/enums/UploadTarget.java`:
- Around line 13-19: The `allowed` field is exposing a mutable EnumSet directly,
which allows external code to modify the upload policy at runtime. Locate the
getter method (likely `getAllowed()`) that returns the `allowed` field and
modify it to return an unmodifiable/immutable view of the set instead of the raw
collection. Use a method like `Collections.unmodifiableSet()` to wrap the
EnumSet before returning it, ensuring the upload policy cannot be tampered with
at runtime.
In `@src/main/java/ditda/backend/global/s3/S3FileService.java`:
- Around line 38-53: The validateUploadedKeys method does not handle null or
blank input values, which can cause NullPointerException instead of a proper
client error response. Add null checks at the beginning of the
validateUploadedKeys method to validate that the keys parameter itself is not
null, and add validation to ensure no individual elements in the keys list are
null or blank strings. For any of these cases, throw a GeneralException with
GeneralErrorCode.INVALID_FILE before proceeding with the existing validation
logic. This ensures null inputs are explicitly rejected as invalid file requests
rather than causing internal errors.
🪄 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.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: f3517650-dea3-4b71-80fa-faa8b74d96e1
📒 Files selected for processing (14)
src/main/java/ditda/backend/domain/commission/core/controller/CommissionController.javasrc/main/java/ditda/backend/domain/commission/core/dto/request/CommissionFilePresignRequest.javasrc/main/java/ditda/backend/domain/commission/core/dto/response/CommissionFilePresignResponse.javasrc/main/java/ditda/backend/domain/commission/core/exception/CommissionErrorCode.javasrc/main/java/ditda/backend/domain/commission/core/facade/CommissionFacade.javasrc/main/java/ditda/backend/domain/commission/core/service/CommissionCreateFileService.javasrc/main/java/ditda/backend/domain/designer/exception/DesignerErrorCode.javasrc/main/java/ditda/backend/domain/designer/service/PortfolioService.javasrc/main/java/ditda/backend/global/apipayload/code/GeneralErrorCode.javasrc/main/java/ditda/backend/global/s3/S3FileService.javasrc/main/java/ditda/backend/global/s3/controller/S3FileController.javasrc/main/java/ditda/backend/global/s3/dto/request/PresignRequest.javasrc/main/java/ditda/backend/global/s3/dto/response/PresignResponse.javasrc/main/java/ditda/backend/global/s3/enums/UploadTarget.java
💤 Files with no reviewable changes (4)
- src/main/java/ditda/backend/domain/commission/core/dto/response/CommissionFilePresignResponse.java
- src/main/java/ditda/backend/domain/commission/core/dto/request/CommissionFilePresignRequest.java
- src/main/java/ditda/backend/domain/commission/core/controller/CommissionController.java
- src/main/java/ditda/backend/domain/commission/core/facade/CommissionFacade.java
fervovita
left a comment
fervovita
left a comment
There was a problem hiding this comment.
s3 패키지에 폴더가 많아져서 너무 flat한 느낌이 들어서, 아래와 같이 변경하는 거는 어떨까요....??
global/s3/
├── controller/ S3FileController
├── enums/ BucketType, S3ContentType, UploadTarget
├── dto/
│ ├── request/ PresignRequest
│ └── response/ PresignResponse, PresignedUpload
├── service/ S3FileService
├── exception/ S3ErrorCode
├── manager/ S3UploadManager
│ S3FileManager
│ S3PresignedUrlGenerator
│ S3UrlResolver
└── config/ S3Properties
(대충 예시입니다!)
확인해보시고 merge 하셔도 될 것 같아요😄
| // 파일 형식 검증 | ||
| S3ContentType type = S3ContentType.from(contentType); | ||
| if (type == null || !target.getAllowed().contains(type)) { | ||
| throw new GeneralException(GeneralErrorCode.UNSUPPORTED_CONTENT_TYPE); |
There was a problem hiding this comment.
UNSUPPORTED_CONTENT_TYPE에러는 API 요청 헤더의 Content-Type을 잘못 보냈을 때 터지는 에러인데, 여기서 같이 사용하게 되면 추후 에러 트래킹시 정확히 에러가 터지는 지점을 파악하기 어려울 것 같습니다.
제 생각에는 s3에도 S3ErrorCode와 같이 정의해서 따로 에러 코드를 관리하는 것이 좋을 것 같아요!
2 participants
🚀 Related issue
Closes #36
#️⃣ Summary
🔧 Changes
POST /api/v1/files/presigned-url로 이전UploadTargetenum으로 생성S3FileService.validateUploadedKeys로 공용화📸 Test Evidence
💬 Reviewer Notes
/instructors/commissions/files/presigned-url→/files/presigned-url,fileKind→target)Summary by CodeRabbit
Release Notes
New Features
Refactor