refactor: TokenAuthenticator 동시성 처리 개선 및 Provider -> Lazy 적용 #260
Conversation
easyhooon
changed the title
Walkthrough프로젝트 작업 지침 문서 두 건을 추가하고, TokenAuthenticator에서 Provider→Lazy 전환 및 synchronized 기반의 check‑then‑refresh 패턴을 도입해 동시 토큰 갱신을 방지하고 에러 처리 및 로깅을 개선했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Authenticator as TokenAuthenticator
participant DataSource as TokenDataSource
participant ReedService
Client->>Authenticator: HTTP Request
Authenticator->>Authenticator: 응답 401 확인
Authenticator->>Authenticator: synchronized 블록 진입 (lock)
alt 다른 요청이 이미 토큰 갱신함
Authenticator->>DataSource: 최신 액세스 토큰 조회
Authenticator->>Client: 새 토큰으로 요청 재시도
else 갱신 필요
Authenticator->>DataSource: 리프레시 토큰 조회
alt 리프레시 토큰 빈값 아님
Authenticator->>ReedService: reedService.value.refreshToken(refreshRequest)
ReedService-->>Authenticator: 새 액세스/리프레시 토큰 반환
Authenticator->>DataSource: 새 토큰 저장
Authenticator->>Client: 새 토큰으로 요청 재시도
else 리프레시 토큰 없음
Authenticator->>DataSource: 토큰 초기화(클리어)
Authenticator->>Client: 인증 실패 반환
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! 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
🤖 Fix all issues with AI agents
In
`@core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenAuthenticator.kt`:
- Around line 24-38: In TokenAuthenticator.authenticate, prevent retries with an
empty access token by adding a guard after obtaining currentToken: if
currentToken is blank/empty, return null to avoid building a retry request with
an empty Authorization header; update the synchronized block around
failedToken/currentToken comparison (variables failedToken and currentToken) so
the existing early-return for when failedToken != currentToken remains but only
proceeds if currentToken is non-empty.
| override fun authenticate(route: Route?, response: Response): Request? { | ||
| return runBlocking { | ||
| try { | ||
| val refreshToken = tokenDataSource.getRefreshToken() | ||
| // 동시 401 응답 시 중복 refresh 방지 (refresh token rotation 대응) | ||
| synchronized(lock) { | ||
| val failedToken = response.request.header("Authorization") | ||
| ?.removePrefix("Bearer ") | ||
| .orEmpty() | ||
|
|
||
| if (refreshToken.isBlank()) { | ||
| Logger.d("TokenAuthenticator", "No refresh token available") | ||
| tokenDataSource.clearTokens() | ||
| return@runBlocking null | ||
| } | ||
| val currentToken = runBlocking { tokenDataSource.getAccessToken() } | ||
|
|
||
| val refreshTokenRequest = RefreshTokenRequest(refreshToken) | ||
| val refreshResponse = serviceProvider().refreshToken(refreshTokenRequest) | ||
| // 다른 요청이 이미 토큰을 갱신한 경우, 새 토큰으로 재시도만 수행 | ||
| if (failedToken != currentToken) { | ||
| return response.request.newBuilder() | ||
| .header("Authorization", "Bearer $currentToken") | ||
| .build() | ||
| } |
There was a problem hiding this comment.
빈 토큰으로 재시도하는 케이스를 막는 게 안전합니다.
currentToken이 비어있는데도 재시도하면 불필요한 401 재요청이 발생할 수 있습니다. 빈 토큰이면 바로 종료하도록 가드 추가를 고려해주세요.
💡 제안 수정
if (failedToken != currentToken) {
+ if (currentToken.isBlank()) {
+ return null
+ }
return response.request.newBuilder()
.header("Authorization", "Bearer $currentToken")
.build()
}🤖 Prompt for AI Agents
In
`@core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenAuthenticator.kt`
around lines 24 - 38, In TokenAuthenticator.authenticate, prevent retries with
an empty access token by adding a guard after obtaining currentToken: if
currentToken is blank/empty, return null to avoid building a retry request with
an empty Authorization header; update the synchronized block around
failedToken/currentToken comparison (variables failedToken and currentToken) so
the existing early-return for when failedToken != currentToken remains but only
proceeds if currentToken is non-empty.
There was a problem hiding this comment.
currentToken이 빈 토큰일 경우의 수가 있을지 고민 좀 해봄
There was a problem hiding this comment.
@easyhooon 좋은 질문입니다. currentToken이 빈 토큰일 수 있는 시나리오를 정리해보면:
- 동시 요청 중 다른 요청의 refresh 실패: 다른 스레드가 토큰 갱신을 시도했지만 실패해서
clearTokens()를 호출한 경우 - 사용자 로그아웃: 401 응답과 이 체크 사이에 로그아웃이 발생한 경우
- 다른 메커니즘으로 토큰 초기화: 예를 들어, 만료된 토큰 감지 등으로 토큰이 클리어된 경우
이런 상황에서 가드 없이 빈 토큰으로 재시도하면:
Authorization: Bearer(빈 값)으로 요청 → 불필요한 401 재발생- 가드가 있으면 바로
null반환 → 불필요한 네트워크 요청 방지
실무에서는 드문 케이스지만, 동시성 이슈나 엣지 케이스를 방어하는 차원에서 가드를 추가하는 것이 안전합니다. 특히 이 PR이 동시성 처리 개선을 목표로 하는 만큼, 이런 엣지 케이스도 함께 처리하면 더 견고한 코드가 될 것 같습니다.
There was a problem hiding this comment.
그나저나 synchronized block vs mutex block
이 상황에선 둘 중에 어떤 블록이 더 적합하다고 생각해?
There was a problem hiding this comment.
@easyhooon 좋은 질문입니다! 이 상황에서는 synchronized block이 더 적합합니다. 이유는:
synchronized가 더 적합한 이유
- 인터페이스 제약:
Authenticator.authenticate()는 OkHttp의 동기(synchronous) 메서드이고, suspend 함수로 만들 수 없습니다 - 블로킹 컨텍스트: OkHttp의 인터셉터 체인은 스레드 블로킹 방식으로 동작합니다
- 이미 runBlocking 사용 중: 어차피 suspend 함수를 호출하기 위해
runBlocking을 사용하고 있어서, Mutex를 써도 결국runBlocking { mutex.withLock { ... } }형태가 되어 오히려 오버헤드만 증가합니다 - 단순한 락 요구사항: 복잡한 코루틴 스케줄링이 필요 없고, 단순히 동시 접근을 막는 것이 목적이므로 JVM 네이티브 synchronized가 더 효율적입니다
Mutex가 유리한 경우
반대로 Mutex는 다음 상황에서 더 좋습니다:
- 전체 로직이 코루틴 기반일 때 (suspend 함수 체인)
- 스레드를 블로킹하지 않고 suspend하고 싶을 때
- 구조화된 동시성(structured concurrency)을 활용하고 싶을 때
결론
현재 코드는 OkHttp의 동기 API 제약 때문에 synchronized가 정답입니다. 만약 Retrofit의 Call Adapter나 다른 방식으로 완전히 코루틴 기반으로 리팩토링한다면 그때 Mutex로 전환을 고려할 수 있습니다.
참고로, 현재 구현은 동시성 처리를 위한 올바른 접근입니다! 👍
There was a problem hiding this comment.
Mutex가 아닌 synchronized 블럭을 선택한 이유가 있을까요?? synchronized 관련 블로그 글은 잘 봤습니다
☺️
@seoyoon513 위 의견에 공감하고 있습니다.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
.claude/CLAUDE.md
Outdated
| @@ -0,0 +1,21 @@ | |||
| # Hime 프로젝트 작업 지침 | |||
| try { | ||
| val refreshToken = tokenDataSource.getRefreshToken() | ||
| // 동시 401 응답 시 중복 refresh 방지 (refresh token rotation 대응) | ||
| synchronized(lock) { |
There was a problem hiding this comment.
Mutex가 아닌 synchronized 블럭을 선택한 이유가 있을까요?? synchronized 관련 블로그 글은 잘 봤습니다
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.claude/CLAUDE.md:
- Around line 13-15: 해당 Markdown 파일의 목록 들여쓰기 규칙이 일관되지 않습니다: 중첩된 리스트 항목은 2칸(스페이스
두 개) 들여쓰기를 사용하도록 `.claude/CLAUDE.md`의 목록 줄(`🤖 Generated with [Claude Code]...`,
`Co-Authored-By: Claude Sonnet...`, 그리고 그 아래의 한국어 문장)을 수정하세요; 각 하위 항목이 최상위 항목보다
정확히 2칸 들여쓰기되도록 정렬하고 탭 대신 스페이스를 사용하며 다른 목록 항목들과 스타일(백틱, 하이픈 등)을 일관되게 유지하세요.
| - `🤖 Generated with [Claude Code](https://claude.com/claude-code)` | ||
| - `Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>` | ||
| - 커밋 작업은 사용자가 직접 수행하는 경우가 많으므로, 요청받지 않은 경우 커밋하지 마세요 |
There was a problem hiding this comment.
마크다운 들여쓰기 수정 필요
정적 분석 도구에서 리스트 항목의 들여쓰기가 일관되지 않다고 지적했습니다. 중첩된 리스트 항목은 2칸 들여쓰기를 사용해야 합니다.
📝 들여쓰기 수정 제안
- **커밋 메시지에서 Claude 관련 문구를 제거합니다**
- 다음 문구들을 커밋 메시지에 포함하지 마세요:
- - `🤖 Generated with [Claude Code](https://claude.com/claude-code)`
- - `Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>`
+ - `🤖 Generated with [Claude Code](https://claude.com/claude-code)`
+ - `Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>`
- 커밋 작업은 사용자가 직접 수행하는 경우가 많으므로, 요청받지 않은 경우 커밋하지 마세요📝 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.
| - `🤖 Generated with [Claude Code](https://claude.com/claude-code)` | |
| - `Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>` | |
| - 커밋 작업은 사용자가 직접 수행하는 경우가 많으므로, 요청받지 않은 경우 커밋하지 마세요 | |
| - `🤖 Generated with [Claude Code](https://claude.com/claude-code)` | |
| - `Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>` | |
| - 커밋 작업은 사용자가 직접 수행하는 경우가 많으므로, 요청받지 않은 경우 커밋하지 마세요 |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
[warning] 14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
[warning] 15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In @.claude/CLAUDE.md around lines 13 - 15, 해당 Markdown 파일의 목록 들여쓰기 규칙이 일관되지
않습니다: 중첩된 리스트 항목은 2칸(스페이스 두 개) 들여쓰기를 사용하도록 `.claude/CLAUDE.md`의 목록 줄(`🤖
Generated with [Claude Code]...`, `Co-Authored-By: Claude Sonnet...`, 그리고 그 아래의
한국어 문장)을 수정하세요; 각 하위 항목이 최상위 항목보다 정확히 2칸 들여쓰기되도록 정렬하고 탭 대신 스페이스를 사용하며 다른 목록 항목들과
스타일(백틱, 하이픈 등)을 일관되게 유지하세요.
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리스 노트
버그 수정
문서
✏️ Tip: You can customize this high-level summary in your review settings.