[FEAT] S3 파일 저장 구조를 Object Key 기반으로 변경 - #166
Conversation
|
Warning Review limit reached
Next review available in: 31 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 (3)
📝 WalkthroughWalkthrough녹음 파일 저장값을 Raw URL에서 S3 Object Key로 변경합니다. 조회 시 Changes녹음 파일 Object Key 전환
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PlayingService
participant AnalysisService
participant HistoryService
participant S3FileService
participant ResponseDTO
Client->>PlayingService: 연주 상세 또는 분석 컨텍스트 조회
PlayingService->>S3FileService: Object Key로 presigned URL 생성
S3FileService-->>PlayingService: recordingFileUrl
PlayingService->>ResponseDTO: URL을 응답에 전달
Client->>AnalysisService: 분석 결과 조회
AnalysisService->>S3FileService: Object Key로 presigned URL 생성
S3FileService-->>AnalysisService: recordingFileUrl
AnalysisService->>ResponseDTO: URL을 응답에 전달
Client->>HistoryService: 히스토리 상세 조회
HistoryService->>S3FileService: Object Key로 presigned URL 생성
S3FileService-->>HistoryService: recordingFileUrl
HistoryService->>ResponseDTO: URL을 응답에 전달
Possibly related PRs
Poem
🚥 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: 2
🧹 Nitpick comments (1)
src/main/java/com/mr/domain/history/service/HistoryService.java (1)
74-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPresigned URL 반환 계약을 성공 경로 테스트로 고정하세요.
이 변경은
recordingObjectKey와userId를createPresignedDownload에 전달하고, 생성된 URL을HistoryDetailResponseDTO에 넣습니다. 성공 테스트가 호출 인자와response.recordingFileUrl()을 검증하지 않으면 잘못된 사용자 ID나 Object Key를 사용해도 통과할 수 있습니다.
HistoryServiceTest의getHistoryDetail성공 케이스와PlayingServiceTest의getPlayingDetail·getAnalysisContext성공 케이스에서 Object Key와 예상 URL을 stub하고,verify와 응답 필드 assertion을 추가하세요. Presigned URL은 사용자와 Object Key의 결합이므로 이 보안 경계를 테스트로 고정하세요. AWS S3 presigned URL 공식 문서도 함께 확인하세요.✅ 성공 케이스 보강 예시
+when(playing.getRecordingObjectKey()) + .thenReturn(RECORDING_OBJECT_KEY); +when(s3FileService.createPresignedDownload( + userId, + RECORDING_OBJECT_KEY +)).thenReturn(RECORDING_FILE_URL); ... +assertThat(response.recordingFileUrl()) + .isEqualTo(RECORDING_FILE_URL); +verify(s3FileService) + .createPresignedDownload(userId, RECORDING_OBJECT_KEY);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/history/service/HistoryService.java` around lines 74 - 80, HistoryServiceTest의 getHistoryDetail 성공 케이스와 PlayingServiceTest의 getPlayingDetail·getAnalysisContext 성공 케이스를 보강하세요. 각 테스트에서 recordingObjectKey와 예상 presigned URL을 stub하고, createPresignedDownload가 올바른 userId와 Object Key로 호출되는지 verify하며 response.recordingFileUrl()이 예상 URL인지 검증하세요.
🤖 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/com/mr/domain/playing/entity/Playing.java`:
- Around line 97-98: Playing의 completeWithMidiData()에서 recordingObjectKey가 공백이
아니고 유효한 Object Key인지 검증한 뒤에만 저장하도록 수정하고, 빈 값은 저장하지 않게 하세요. 마이그레이션에서는 기존
recording_object_key의 쿼리 문자열과 URL 인코딩 변수를 제거하거나, 변경된 데이터가 있음을 로그로 남기세요.
createPresignedDownload() 조회 경로와 일관되게 처리하되, http://example.com/file.webm 같은 URL을
file/webm으로 변환하지 않도록 별도 방어 조건을 유지하세요.
In
`@src/main/resources/db/migration/V5__migrate_recording_file_url_to_object_key.sql`:
- Around line 16-24: Update the validation section of the migration to check
whether any rows match the invalid recording_object_key patterns, and raise an
exception when such rows exist before reaching COMMIT. Use an IF EXISTS check
around the existing playing query so Flyway aborts and rolls back on validation
failure, while preserving the successful zero-row commit path.
---
Nitpick comments:
In `@src/main/java/com/mr/domain/history/service/HistoryService.java`:
- Around line 74-80: HistoryServiceTest의 getHistoryDetail 성공 케이스와
PlayingServiceTest의 getPlayingDetail·getAnalysisContext 성공 케이스를 보강하세요. 각 테스트에서
recordingObjectKey와 예상 presigned URL을 stub하고, createPresignedDownload가 올바른
userId와 Object Key로 호출되는지 verify하며 response.recordingFileUrl()이 예상 URL인지 검증하세요.
🪄 Autofix
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: e3011968-a621-4eb8-9304-ca2f838be950
📒 Files selected for processing (17)
src/main/java/com/mr/domain/analysis/dto/res/AnalysisResultResponseDTO.javasrc/main/java/com/mr/domain/analysis/service/AnalysisService.javasrc/main/java/com/mr/domain/history/dto/res/HistoryDetailResponseDTO.javasrc/main/java/com/mr/domain/history/service/HistoryService.javasrc/main/java/com/mr/domain/playing/dto/res/AnalysisContextResponse.javasrc/main/java/com/mr/domain/playing/dto/res/PlayingDetailResponse.javasrc/main/java/com/mr/domain/playing/entity/Playing.javasrc/main/java/com/mr/domain/playing/service/PlayingService.javasrc/main/java/com/mr/global/file/s3/dto/ValidatedFile.javasrc/main/java/com/mr/global/file/s3/service/S3FileService.javasrc/main/resources/db/migration/V5__migrate_recording_file_url_to_object_key.sqlsrc/test/java/com/mr/domain/analysis/service/AnalysisServiceTest.javasrc/test/java/com/mr/domain/history/service/HistoryServiceTest.javasrc/test/java/com/mr/domain/playing/entity/PlayingTest.javasrc/test/java/com/mr/domain/playing/service/PlayingServiceTest.javasrc/test/java/com/mr/global/file/s3/service/RecordingObjectKeyGeneratorTest.javasrc/test/java/com/mr/global/file/s3/service/S3FileServiceTest.java
💤 Files with no reviewable changes (3)
- src/test/java/com/mr/global/file/s3/service/S3FileServiceTest.java
- src/main/java/com/mr/global/file/s3/service/S3FileService.java
- src/main/java/com/mr/global/file/s3/dto/ValidatedFile.java
There was a problem hiding this comment.
전체적으로 리팩터 방향이 깔끔합니다.
- 마이그레이션 검증:
V5__migrate_recording_file_url_to_object_key.sql에서 URL→Object Key 변환 후DO $$ ... IF EXISTS ... RAISE EXCEPTION으로 검증해서, 변환이 잘못된 값이 남아있으면 Flyway가 트랜잭션째로 롤백하도록 만들어두신 부분 좋습니다. 빈 문자열 케이스(btrim(...) = '')까지 같이 잡아주셔서 꼼꼼했어요. - Presigned URL 발급 전 소유권 검증:
PlayingService/HistoryService/AnalysisService세 곳 다s3FileService.createPresignedDownload()호출 전에validatePlayingOwner/validateOwner로 소유권을 먼저 확인하고,S3FileService내부에서도objectKeyGenerator.belongsToOwner()로 한 번 더 검증하는 이중 방어가 되어 있습니다. 다른 유저의 recordingObjectKey를 조작해서 presigned URL을 받아가는 경로가 없네요. - 죽은 코드 정리:
extractObjectKey/buildFileUrl같은 URL 기반 레거시 로직을 깔끔하게 삭제하고, 관련 테스트(mockFileUrl등)까지 같이 정리하신 점도 좋습니다. 관련 있는 서비스/DTO/테스트가 다 일관되게 같이 수정돼서 반쯤 남은 코드가 없어요.
이대로 머지하셔도 될 것 같습니다 👍
rkdehdrbs7885-oss
left a comment
rkdehdrbs7885-oss
left a comment
There was a problem hiding this comment.
URL을 DB에 저장하던 기존 방식에서 Object Key 기반으로 뼈대를 바꾸고 조회 시점에 Presigned URL을 발급하도록 잘 변경한 것 같습니다!
수고하셨습니다!
3 participants
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
recording_file_url데이터를recording_object_key로 변환ValidatedFile을 Object Key 기반 DTO로 변경하고 Raw URL 의존 로직 제거🔥 리뷰 요청 사항
✅ 체크리스트
📎 참고 사항
Summary by CodeRabbit
개선 사항
테스트