[REFACTOR] 백킹트랙 오디오 S3 업로드 및 조회 기능 추가 - #188
Conversation
|
Warning Review limit reached
Next review available in: 13 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 (4)
📝 WalkthroughWalkthrough백킹 트랙 오디오 저장 방식을 URL에서 S3 Object Key로 변경했습니다. 파일 유형별 업로드·검증·다운로드 URL을 지원합니다. 연주, 분석 결과, 히스토리 응답에 presigned URL을 전달합니다. ChangesS3 오디오 계약과 검증
백킹 트랙 오디오 업로드
연주 오디오 응답
분석 및 히스토리 응답
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PlayingService
participant S3FileService
participant PlayingStartResponse
Client->>PlayingService: 연주 시작 요청
PlayingService->>S3FileService: 녹음 및 백킹 트랙 다운로드 URL 생성
S3FileService-->>PlayingService: presigned download URLs
PlayingService->>PlayingStartResponse: 오디오 URL 전달
PlayingStartResponse-->>Client: 연주 시작 응답
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/mr/domain/backingtrack/service/BackingTrackService.java (2)
67-86: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win정규화한
audioObjectKey를 저장하세요.공백 문자열은 Lines 67-70에서
null로 정규화됩니다. 하지만 Line 86은 원본request.audioObjectKey()를 저장합니다. 따라서" "값은 검증 없이 DB에 저장되고 이후 조회에서는 오디오가 없는 값처럼 처리됩니다.수정 예시
- request.audioObjectKey(), + audioObjectKey,🤖 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/backingtrack/service/BackingTrackService.java` around lines 67 - 86, BackingTrackService의 create 흐름에서 검증에 사용한 정규화된 audioObjectKey를 저장하도록 BackingTrack.create 호출의 원본 request.audioObjectKey() 인자를 audioObjectKey로 교체하세요. 공백 문자열은 null로 저장되고, 유효한 키는 기존 값 그대로 유지되어야 합니다.
139-149: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift백킹트랙 수정 API가
audioObjectKey를 무시합니다. 수정 요청은audioObjectKey를 허용하지만 서비스와 엔티티는 값을 검증하거나 저장하지 않습니다. 클라이언트는 성공 응답을 받아도 오디오가 변경되지 않으며, 새로 업로드한 S3 객체는 연결되지 않은 상태로 남습니다.
src/main/java/com/mr/domain/backingtrack/service/BackingTrackService.java#L139-L149: 오디오 교체를 지원하면audioObjectKey를 정규화하고S3FileType.BACKING_TRACK로 검증한 뒤 엔티티에 전달하세요.src/main/java/com/mr/domain/backingtrack/dto/req/BackingTrackSaveRequestDTO.java#L49-L50: 오디오 교체를 지원하지 않으면 생성 요청과 수정 요청 DTO를 분리하고 수정 요청에서audioObjectKey를 제거하세요.src/main/java/com/mr/domain/backingtrack/entity/BackingTrack.java#L159-L168: 오디오 교체를 지원하면 검증된 Object Key를 저장하는 전용 메서드 또는updateTrackInfo인자를 복원하세요.🤖 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/backingtrack/service/BackingTrackService.java` around lines 139 - 149, 수정 API가 audioObjectKey를 처리하지 않는 문제를 해결하세요. src/main/java/com/mr/domain/backingtrack/service/BackingTrackService.java:139-149에서는 교체를 지원할 경우 audioObjectKey를 정규화하고 S3FileType.BACKING_TRACK으로 검증한 뒤 BackingTrack에 전달하세요. src/main/java/com/mr/domain/backingtrack/entity/BackingTrack.java:159-168에서는 검증된 키를 저장하도록 전용 메서드 또는 updateTrackInfo 인자를 추가하세요. 교체를 지원하지 않는 설계라면 src/main/java/com/mr/domain/backingtrack/dto/req/BackingTrackSaveRequestDTO.java:49-50에서 생성·수정 DTO를 분리하고 수정 요청에서 audioObjectKey를 제거하세요.src/main/java/com/mr/global/file/s3/service/S3FileService.java (1)
72-74: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftS3 객체 검증을 DB 트랜잭션 밖으로 이동하세요.
createBackingTrack은@Transactional상태에서validateUploadedFile을 호출합니다. 이 호출은s3Client.headObject네트워크 요청을 수행합니다. S3 지연 또는 재시도가 발생하면 DB 트랜잭션과 커넥션이 불필요하게 유지됩니다.S3 검증을 트랜잭션 시작 전에 수행하세요. 검증 후 DB 조회와 저장만 별도 트랜잭션에서 수행하세요. Spring 트랜잭션 경계와 AWS SDK timeout 설정도 함께 확인하세요.
🤖 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/global/file/s3/service/S3FileService.java` around lines 72 - 74, createBackingTrack에서 validateUploadedFile(S3 headObject 검증)을 `@Transactional` DB 작업과 분리하세요. 검증은 트랜잭션 시작 전에 수행하고, 이후 DB 조회·저장만 별도 Spring 트랜잭션에서 실행되도록 트랜잭션 경계를 조정하세요. 또한 s3Client의 연결·읽기·API 호출 timeout 및 재시도 설정을 확인해 지연 시 장시간 대기하지 않도록 하세요.
🧹 Nitpick comments (2)
src/test/java/com/mr/domain/playing/service/PlayingServiceTest.java (1)
650-663: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win백킹트랙 소유자와 요청 사용자가 다른 경우를 검증하세요.
현재 fixture는 백킹트랙 소유자와 요청 사용자를 같은
user로 설정합니다. 따라서createPresignedDownload에 요청 사용자 ID를 전달하는 회귀를 검출하지 못합니다.공개 백킹트랙의 소유자를 별도 사용자로 설정하세요.
ownerId,S3FileType.BACKING_TRACK,BACKING_TRACK_OBJECT_KEY로 호출했는지 검증하세요. 응답의audioFileUrl도BACKING_TRACK_FILE_URL인지 검증하세요. 필요하면 Mockitoverify문서를 참고하세요.테스트 보강 예시
+Long backingTrackOwnerId = 2L; +User backingTrackOwner = mock(User.class); +given(backingTrack.getUser()).willReturn(backingTrackOwner); +given(backingTrackOwner.getUserId()).willReturn(backingTrackOwnerId); +given(s3FileService.createPresignedDownload( + backingTrackOwnerId, + S3FileType.BACKING_TRACK, + BACKING_TRACK_OBJECT_KEY +)).willReturn(BACKING_TRACK_FILE_URL); + +assertThat(response.backingTrack().audioFileUrl()) + .isEqualTo(BACKING_TRACK_FILE_URL); +verify(s3FileService).createPresignedDownload( + backingTrackOwnerId, + S3FileType.BACKING_TRACK, + BACKING_TRACK_OBJECT_KEY +);Also applies to: 840-853
🤖 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/test/java/com/mr/domain/playing/service/PlayingServiceTest.java` around lines 650 - 663, Update the public backing-track fixture in the relevant PlayingServiceTest case so backingTrack.getUser() returns a separate owner whose ownerId is distinct from the requesting userId. Verify s3FileService.createPresignedDownload is called with ownerId, S3FileType.BACKING_TRACK, and BACKING_TRACK_OBJECT_KEY, and assert the response audioFileUrl equals BACKING_TRACK_FILE_URL.src/test/java/com/mr/global/file/s3/service/RecordingObjectKeyGeneratorTest.java (1)
248-328: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
BACKING_TRACK네임스페이스 경계도 테스트하세요.현재 소유자 검증은
RECORDING경로만 검사합니다.backing-tracks/{userId}/...생성과RECORDING키를BACKING_TRACK으로 검증할 때false를 반환하는 경우를 추가하세요.이 테스트가 없으면 파일 유형 prefix 변경 시 백킹트랙 업로드 또는 다운로드 권한 검증이 깨져도 감지하지 못합니다.
🤖 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/test/java/com/mr/global/file/s3/service/RecordingObjectKeyGeneratorTest.java` around lines 248 - 328, Extend the belongsToOwner tests around objectKeyGenerator to cover the BACKING_TRACK namespace: verify a backing-tracks/{userId}/... key generated for BACKING_TRACK returns true, and verify a RECORDING-prefixed key validated as BACKING_TRACK returns false. Use the existing FILE_TYPE and object-key test structure while adding the appropriate backing-track file type value.
🤖 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/analysis/service/AnalysisService.java`:
- Around line 125-127: Update the recording download URL logic in
AnalysisService.java lines 125-127 and HistoryService.java lines 82-84 to apply
the same null-or-blank recordingObjectKey check used for the backing track; only
call createPresignedDownload when the key is present, otherwise preserve the URL
as null.
In
`@src/main/resources/db/migration/V7__migrate_backing_track_audio_file_url_to_object_key.sql`:
- Around line 6-13: Update the V7 migration so legacy backing-track objects
under recordings/... are moved or copied into the backing-tracks/{ownerId}/...
prefix defined by S3FileType.BACKING_TRACK, and update
backing_track.audio_object_key to the new key in the same migration. Preserve
existing URL/query-string normalization while ensuring migrated keys satisfy the
expectedPrefix validation.
---
Outside diff comments:
In `@src/main/java/com/mr/domain/backingtrack/service/BackingTrackService.java`:
- Around line 67-86: BackingTrackService의 create 흐름에서 검증에 사용한 정규화된
audioObjectKey를 저장하도록 BackingTrack.create 호출의 원본 request.audioObjectKey() 인자를
audioObjectKey로 교체하세요. 공백 문자열은 null로 저장되고, 유효한 키는 기존 값 그대로 유지되어야 합니다.
- Around line 139-149: 수정 API가 audioObjectKey를 처리하지 않는 문제를 해결하세요.
src/main/java/com/mr/domain/backingtrack/service/BackingTrackService.java:139-149에서는
교체를 지원할 경우 audioObjectKey를 정규화하고 S3FileType.BACKING_TRACK으로 검증한 뒤 BackingTrack에
전달하세요.
src/main/java/com/mr/domain/backingtrack/entity/BackingTrack.java:159-168에서는 검증된
키를 저장하도록 전용 메서드 또는 updateTrackInfo 인자를 추가하세요. 교체를 지원하지 않는 설계라면
src/main/java/com/mr/domain/backingtrack/dto/req/BackingTrackSaveRequestDTO.java:49-50에서
생성·수정 DTO를 분리하고 수정 요청에서 audioObjectKey를 제거하세요.
In `@src/main/java/com/mr/global/file/s3/service/S3FileService.java`:
- Around line 72-74: createBackingTrack에서 validateUploadedFile(S3 headObject
검증)을 `@Transactional` DB 작업과 분리하세요. 검증은 트랜잭션 시작 전에 수행하고, 이후 DB 조회·저장만 별도 Spring
트랜잭션에서 실행되도록 트랜잭션 경계를 조정하세요. 또한 s3Client의 연결·읽기·API 호출 timeout 및 재시도 설정을 확인해 지연
시 장시간 대기하지 않도록 하세요.
---
Nitpick comments:
In `@src/test/java/com/mr/domain/playing/service/PlayingServiceTest.java`:
- Around line 650-663: Update the public backing-track fixture in the relevant
PlayingServiceTest case so backingTrack.getUser() returns a separate owner whose
ownerId is distinct from the requesting userId. Verify
s3FileService.createPresignedDownload is called with ownerId,
S3FileType.BACKING_TRACK, and BACKING_TRACK_OBJECT_KEY, and assert the response
audioFileUrl equals BACKING_TRACK_FILE_URL.
In
`@src/test/java/com/mr/global/file/s3/service/RecordingObjectKeyGeneratorTest.java`:
- Around line 248-328: Extend the belongsToOwner tests around objectKeyGenerator
to cover the BACKING_TRACK namespace: verify a backing-tracks/{userId}/... key
generated for BACKING_TRACK returns true, and verify a RECORDING-prefixed key
validated as BACKING_TRACK returns false. Use the existing FILE_TYPE and
object-key test structure while adding the appropriate backing-track file type
value.
🪄 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: 8c32f84e-7242-49c1-ad67-1e99e58480ab
📒 Files selected for processing (22)
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/backingtrack/controller/BackingTrackController.javasrc/main/java/com/mr/domain/backingtrack/dto/req/BackingTrackSaveRequestDTO.javasrc/main/java/com/mr/domain/backingtrack/dto/req/BackingTrackUploadUrlRequest.javasrc/main/java/com/mr/domain/backingtrack/dto/res/BackingTrackUploadUrlResponse.javasrc/main/java/com/mr/domain/backingtrack/entity/BackingTrack.javasrc/main/java/com/mr/domain/backingtrack/service/BackingTrackService.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/PlayingStartResponse.javasrc/main/java/com/mr/domain/playing/service/PlayingService.javasrc/main/java/com/mr/global/file/s3/enums/S3FileType.javasrc/main/java/com/mr/global/file/s3/service/S3FileService.javasrc/main/java/com/mr/global/file/s3/service/S3ObjectKeyGenerator.javasrc/main/resources/db/migration/V7__migrate_backing_track_audio_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/service/PlayingServiceTest.javasrc/test/java/com/mr/global/file/s3/service/RecordingObjectKeyGeneratorTest.javasrc/test/java/com/mr/global/file/s3/service/S3FileServiceTest.java
p1001q
left a comment
p1001q
left a comment
There was a problem hiding this comment.
S3FileType 기반으로 recording/backing-track 두 파일 타입을 깔끔하게 통합하신 것도 좋았고
특히 백킹트랙은 소유자(제작자)와 요청자가 다를 수 있다는 걸 놓치지 않고
presigned URL 발급 시 소유자 ID를 정확히 넘기신 부분이 좋네요~
CodeRabbit이 짚은 마이그레이션 이슈 반영도 확인했습니다. 고생하셨습니다!
3 participants
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
🔥 리뷰 요청 사항
✅ 체크리스트
📎 참고 사항
백킹트랙 오디오 업로드 흐름
Presigned PUT URL 발급 → 프론트에서 S3 직접 업로드 → objectKey를 백킹트랙 생성 API에 전달 → S3 객체 검증 → DB에 Object Key 저장조회 시에는 DB에 저장된 Object Key를 기반으로 Presigned GET URL을 생성하여 클라이언트에 반환합니다.
Summary by CodeRabbit