-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: TokenAuthenticator 동시성 처리 개선 및 Provider -> Lazy 적용 #260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Reed 프로젝트 작업 지침 | ||
|
|
||
| ## 빌드 관련 | ||
|
|
||
| - **빌드는 사용자가 직접 수행합니다** | ||
| - 기능 작업 완료 후 빌드를 자동으로 실행하지 마세요 (시간이 오래 걸림) | ||
| - 빌드가 필요한 경우 사용자에게 알리고 사용자가 직접 실행하도록 합니다 | ||
|
|
||
| ## 커밋 관련 | ||
|
|
||
| - **커밋 메시지에서 Claude 관련 문구를 제거합니다** | ||
| - 다음 문구들을 커밋 메시지에 포함하지 마세요: | ||
| - `🤖 Generated with [Claude Code](https://claude.com/claude-code)` | ||
| - `Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>` | ||
| - 커밋 작업은 사용자가 직접 수행하는 경우가 많으므로, 요청받지 않은 경우 커밋하지 마세요 | ||
|
|
||
| ## MCP 설정 관련 | ||
|
|
||
| - **Claude Code CLI의 MCP 설정 파일 위치** | ||
| - Claude Desktop이 아니라 **Claude Code CLI**를 사용 중입니다 | ||
| - MCP 설정은 `~/.claude.json`의 `projects` 섹션에서 프로젝트별로 관리됩니다 | ||
| - Claude Desktop 설정 파일(`~/Library/Application Support/Claude/claude_desktop_config.json`)을 수정하지 마세요 | ||
| - **Figma MCP 설정 경로** | ||
| - `~/.claude.json` → `projects` → `/Users/medi/AndroidStudioProjects/YeoBee-Android` → `mcpServers` → `figma` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # 코딩 가이드 | ||
|
|
||
| ## 코드 작성 원칙 | ||
|
|
||
| - 한글 주석 사용 | ||
| - Kotlin 코딩 컨벤션 준수 | ||
| - 기존 코드 스타일 유지 | ||
| - 파일 끝에 빈 줄(newline) 추가 | ||
|
|
||
| ## Compose 관련 | ||
|
|
||
| - **Composable 함수 내 Collection 타입** | ||
| - `List`, `Set`, `Map` 등의 Collection 대신 `ImmutableList`, `ImmutableSet`, `ImmutableMap` 사용 | ||
| - `kotlinx.collections.immutable` 라이브러리 사용 | ||
| - 예시: | ||
| ```kotlin | ||
| // ❌ 사용하지 않음 | ||
| @Composable | ||
| fun TripList(trips: List<Trip>) { ... } | ||
|
|
||
| // ✅ 사용 | ||
| @Composable | ||
| fun TripList(trips: ImmutableList<Trip>) { ... } | ||
| ``` | ||
| - 변환 시 `toImmutableList()`, `persistentListOf()` 등 사용 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,6 @@ import com.ninecraft.booket.core.network.request.RefreshTokenRequest | |
| import com.ninecraft.booket.core.network.service.ReedService | ||
| import com.orhanobut.logger.Logger | ||
| import dev.zacsweers.metro.Inject | ||
| import dev.zacsweers.metro.Provider | ||
| import dev.zacsweers.metro.SingleIn | ||
| import kotlinx.coroutines.runBlocking | ||
| import okhttp3.Authenticator | ||
|
|
@@ -18,38 +17,53 @@ import okhttp3.Route | |
| @Inject | ||
| class TokenAuthenticator( | ||
| private val tokenDataSource: TokenDataSource, | ||
| private val serviceProvider: Provider<ReedService>, | ||
| private val reedService: Lazy<ReedService>, | ||
| ) : Authenticator { | ||
| private val lock = Any() | ||
|
|
||
| override fun authenticate(route: Route?, response: Response): Request? { | ||
| return runBlocking { | ||
| try { | ||
| val refreshToken = tokenDataSource.getRefreshToken() | ||
| // 동시 401 응답 시 중복 refresh 방지 (refresh token rotation 대응) | ||
| synchronized(lock) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mutex가 아닌 synchronized 블럭을 선택한 이유가 있을까요?? synchronized 관련 블로그 글은 잘 봤습니다 |
||
| 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() | ||
| } | ||
|
Comment on lines
24
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 빈 토큰으로 재시도하는 케이스를 막는 게 안전합니다. 💡 제안 수정 if (failedToken != currentToken) {
+ if (currentToken.isBlank()) {
+ return null
+ }
return response.request.newBuilder()
.header("Authorization", "Bearer $currentToken")
.build()
}🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. currentToken이 빈 토큰일 경우의 수가 있을지 고민 좀 해봄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
이런 상황에서 가드 없이 빈 토큰으로 재시도하면:
실무에서는 드문 케이스지만, 동시성 이슈나 엣지 케이스를 방어하는 차원에서 가드를 추가하는 것이 안전합니다. 특히 이 PR이 동시성 처리 개선을 목표로 하는 만큼, 이런 엣지 케이스도 함께 처리하면 더 견고한 코드가 될 것 같습니다.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그나저나 synchronized block vs mutex block 이 상황에선 둘 중에 어떤 블록이 더 적합하다고 생각해? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
synchronized가 더 적합한 이유
Mutex가 유리한 경우반대로 Mutex는 다음 상황에서 더 좋습니다:
결론현재 코드는 OkHttp의 동기 API 제약 때문에 참고로, 현재 구현은 동시성 처리를 위한 올바른 접근입니다! 👍
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@seoyoon513 위 의견에 공감하고 있습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| tokenDataSource.apply { | ||
| setAccessToken(refreshResponse.accessToken) | ||
| setRefreshToken(refreshResponse.refreshToken) | ||
| } | ||
| return runBlocking { | ||
| try { | ||
| val refreshToken = tokenDataSource.getRefreshToken() | ||
|
|
||
| Logger.d("TokenAuthenticator", "Token refreshed successfully") | ||
| if (refreshToken.isBlank()) { | ||
| Logger.d("No refresh token available") | ||
| tokenDataSource.clearTokens() | ||
| return@runBlocking null | ||
| } | ||
|
|
||
| response.request.newBuilder() | ||
| .header("Authorization", "Bearer ${refreshResponse.accessToken}") | ||
| .build() | ||
| } catch (e: Exception) { | ||
| Logger.e("TokenAuthenticator", e.message) | ||
| tokenDataSource.clearTokens() | ||
| val refreshResponse = reedService.value.refreshToken(RefreshTokenRequest(refreshToken)) | ||
|
|
||
| tokenDataSource.apply { | ||
| setAccessToken(refreshResponse.accessToken) | ||
| setRefreshToken(refreshResponse.refreshToken) | ||
| } | ||
|
|
||
| Logger.d("Token refreshed successfully") | ||
|
|
||
| // refresh token이 만료되었거나 잘못된 경우, 재시도하지 않음 | ||
| return@runBlocking null | ||
| response.request.newBuilder() | ||
| .header("Authorization", "Bearer ${refreshResponse.accessToken}") | ||
| .build() | ||
| } catch (e: Exception) { | ||
| Logger.e(e, "Token refresh failed") | ||
| tokenDataSource.clearTokens() | ||
| return@runBlocking null | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마크다운 들여쓰기 수정 필요
정적 분석 도구에서 리스트 항목의 들여쓰기가 일관되지 않다고 지적했습니다. 중첩된 리스트 항목은 2칸 들여쓰기를 사용해야 합니다.
📝 들여쓰기 수정 제안
📝 Committable suggestion
🧰 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