[FEAT] 분석 마디 선택 정보 조회 API 구현 - #161
Conversation
📝 WalkthroughWalkthrough분석 마디 계산을 Changes분석 마디 계산 및 분석 요청 연계
분석 컨텍스트 조회 API
로컬 OAuth 및 S3 설정
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant PlayingController
participant PlayingService
participant AnalysisBarCalculator
participant AnalysisContextResponse
Client->>PlayingController: GET /api/playings/{playingId}/analysis-context
PlayingController->>PlayingService: getAnalysisContext(userId, playingId)
PlayingService->>AnalysisBarCalculator: calculate(playing)
AnalysisBarCalculator-->>PlayingService: totalBars
PlayingService->>AnalysisContextResponse: from(playing, totalBars)
AnalysisContextResponse-->>PlayingService: 분석 컨텍스트
PlayingService-->>PlayingController: AnalysisContextResponse
PlayingController-->>Client: ApiResponse
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: 1
🧹 Nitpick comments (1)
src/test/java/com/mr/domain/playing/service/PlayingServiceTest.java (1)
1000-1015: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win완료되지 않은 연주 거부 경로를 테스트하세요.
현재 테스트는 성공 경로에서
validateCompleted()호출만 확인합니다.validateCompleted()가 예외를 던질 때getAnalysisContext가 그 예외를 반환하고analysisBarCalculator를 호출하지 않는 테스트를 추가하세요. 이 테스트는 완료 상태 검증 순서의 회귀를 방지합니다.🤖 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 1000 - 1015, PlayingServiceTest의 getAnalysisContext 테스트에 validateCompleted()가 GeneralException을 던지는 거부 경로를 추가하세요. playing 소유자 검증은 통과하도록 설정한 뒤 validateCompleted() 예외가 전파되는지 확인하고, analysisBarCalculator.calculate(any())가 호출되지 않았는지도 검증하세요.
🤖 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/AnalysisBarCalculator.java`:
- Around line 39-41: Update the value parsing in AnalysisBarCalculator to call
split with a negative limit so trailing empty tokens are preserved; continue
rejecting any result whose parts length is not exactly two, ensuring inputs such
as "4/4/" produce ANALYSIS_INVALID_REQUEST. Add coverage for the
trailing-delimiter case if tests for this parsing path exist.
---
Nitpick comments:
In `@src/test/java/com/mr/domain/playing/service/PlayingServiceTest.java`:
- Around line 1000-1015: PlayingServiceTest의 getAnalysisContext 테스트에
validateCompleted()가 GeneralException을 던지는 거부 경로를 추가하세요. playing 소유자 검증은 통과하도록
설정한 뒤 validateCompleted() 예외가 전파되는지 확인하고,
analysisBarCalculator.calculate(any())가 호출되지 않았는지도 검증하세요.
🪄 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: a95a3595-72fb-452e-9bd7-5eb902d44c76
📒 Files selected for processing (11)
MR_config/local/application.example.ymlsrc/main/java/com/mr/domain/analysis/factory/AnalysisRequestFactory.javasrc/main/java/com/mr/domain/analysis/service/AnalysisBarCalculator.javasrc/main/java/com/mr/domain/playing/controller/PlayingController.javasrc/main/java/com/mr/domain/playing/dto/res/AnalysisContextResponse.javasrc/main/java/com/mr/domain/playing/service/PlayingService.javasrc/main/resources/application.ymlsrc/test/java/com/mr/domain/analysis/service/AnalysisBarCalculatorTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisRequestFactoryTest.javasrc/test/java/com/mr/domain/playing/controller/PlayingControllerTest.javasrc/test/java/com/mr/domain/playing/service/PlayingServiceTest.java
| String[] parts = value.split("/"); | ||
| if (parts.length != 2) { | ||
| throw new NumberFormatException(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Java String.split(String regex) trailing empty strings official Java documentation
💡 Result:
According to official Java documentation, the String.split(String regex) method discards trailing empty strings by default [1][2]. This behavior occurs because the single-argument split(String regex) method is equivalent to calling the two-argument split(String regex, int limit) method with a limit argument of zero [1][3]. When the limit parameter is set to zero, the pattern is applied as many times as possible, but trailing empty strings are discarded from the resulting array [1][4]. If you need to include trailing empty strings, you can use the overloaded split(String regex, int limit) method with a negative value for the limit parameter [1][5]. When the limit is negative, the pattern is applied as many times as possible and the array can have any length, which preserves trailing empty strings [1][2].
Citations:
- 1: https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/lang/String.html
- 2: https://docs.oracle.com/javase/8/docs/api/java/lang/String.html?is-external=true
- 3: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/String.html
- 4: https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/lang/String.html
- 5: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/String.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find file =="
fd -a 'AnalysisBarCalculator\.java$' . || true
echo "== lines around target =="
if [ -f src/main/java/com/mr/domain/analysis/service/AnalysisBarCalculator.java ]; then
wc -l src/main/java/com/mr/domain/analysis/service/AnalysisBarCalculator.java
sed -n '1,120p' src/main/java/com/mr/domain/analysis/service/AnalysisBarCalculator.java | cat -n
fi
echo "== related tests/usages for AnalysisBarCalculator =="
rg -n "AnalysisBarCalculator|analyse|calculate|ANALYSIS_INVALID_REQUEST|split\\(\"/\"|--2|split\\(\"/\\)" . -g '*.java' -g '*.kt' || true
echo "== Java split behavior probe if javac/java present =="
if command -v javac >/dev/null 2>&1 && command -v java >/dev/null 2>&1; then
tmp="$(mktemp -d)"
cat > "$tmp/Probe.java" <<'EOF'
import java.util.Arrays;
public class Probe {
public static void main(String[] args) {
for (String v : new String[]{"4/4", "4/4/", "/4/4", ""}) {
System.out.println(Arrays.toString(v.split("/")) + " :: " + Arrays.toString(v.split("/", -1)));
}
}
}
EOF
javac "$tmp/Probe.java"
java -cp "$tmp" Probe
else
echo "javac/java not available"
fiRepository: Musereview/BE
Length of output: 17439
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Repository files containing AnalysisBarCalculator or related error =="
git ls-files | rg 'AnalysisBarCalculator|[^/]AnalysisBar.*|.*Bar.*Calculator.*' || true
rg -n "AnalysisBarCalculator|ANALYSIS_INVALID_REQUEST|AnalysisBarCalculatorTest|analyzeBar|calculateBar" . || true
echo "== Java documentation slice from local JDK if available =="
if [ -n "${JAVA_HOME:-}" ] && [ -d "$JAVA_HOME" ]; then
rg -n "discards trailing empty strings|limit argument of zero|Arrays.toString\(this.split.*limit" "$JAVA_HOME"/docs -g '*.html' -g '*.javadoc' 2>/dev/null | head -20 || true
fiRepository: Musereview/BE
Length of output: 3778
끝 구분자가 있는 박자표를 거부하세요.
String.split("/")는 기본값으로 뒤에 남은 빈 토큰을 버립니다. 따라서 "4/4/"는 ["4", "4"]로 해석되어 유효한 4/4가 됩니다. 이를 입력 값 자체의 잘못된 형식으로 처리해 ANALYSIS_INVALID_REQUEST를 반환하려면 split("/", -1)로 파싱하세요. 이 경로를 테스트도 추가하면 좋습니다. 관련 문서는 Java 기본 API의 String.split(String) 문서의 limit 설명을 참고하세요.
수정 예시
- String[] parts = value.split("/");
+ String[] parts = value.split("/", -1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String[] parts = value.split("/"); | |
| if (parts.length != 2) { | |
| throw new NumberFormatException(); | |
| String[] parts = value.split("/", -1); | |
| if (parts.length != 2) { | |
| throw new NumberFormatException(); |
🤖 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/analysis/service/AnalysisBarCalculator.java`
around lines 39 - 41, Update the value parsing in AnalysisBarCalculator to call
split with a negative limit so trailing empty tokens are preserved; continue
rejecting any result whose parts length is not exactly two, ensuring inputs such
as "4/4/" produce ANALYSIS_INVALID_REQUEST. Add coverage for the
trailing-delimiter case if tests for this parsing path exist.
on1yoneprivate
left a comment
on1yoneprivate
left a comment
There was a problem hiding this comment.
수고하셨습니다.
[참고]
현재 recordingFileUrl은 raw S3 URL이 그대로 응답되고 있는데, private 객체인 경우 프론트에서 접근 시 AccessDenied가 발생할 수 있어요!
다만 이 부분은 제가 현재 작업 중인 presigned GET URL 발급 PR #159에서 함께 수정하고 있어 은우님 PR이 먼저 머지 후 제가 반영해서 수정할게요!
p1001q
left a comment
p1001q
left a comment
There was a problem hiding this comment.
Presigned URL 부분은 원정님 담당이라 리뷰 스코프에서 뺐고 나머지 로직(totalBars 계산 일관성, 소유권/완료 상태 검증 흐름)은 문제없어 보이네용 바로 승인하고 머지 하겠습니다~
4 participants
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
🔥 리뷰 요청 사항
totalBars계산 기준이 분석 요청의 마디 범위 검증과 일관되는지✅ 체크리스트
📎 참고 사항
PLAYING_404_03을 반환합니다. (추후 백킹트랙이 없어도 분석이 가능하게 디벨롭할 경우, 해당 에러를 별도로 처리해야 합니다.)null로 반환됩니다.Summary by CodeRabbit
새 기능
개선
테스트