Skip to content

feat: 로그아웃, 회원탈퇴 api연결 - #61

Merged
JiwonLee42 merged 12 commits into
developfrom
feature/56-login-api
Jul 31, 2026
Merged

feat: 로그아웃, 회원탈퇴 api연결#61
JiwonLee42 merged 12 commits into
developfrom
feature/56-login-api

Conversation

@dada4679

@dada4679 dada4679 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

📄 작업 내용 요약

-로그아웃, 회원탈퇴 api연결

📎 Issue 번호


✅ 작업 목록

  • 기능 구현
  • 코드 리뷰 반영
  • 테스트 코드 작성
  • 문서 업데이트

📝 기타 참고사항

Summary by CodeRabbit

  • 새로운 기능

    • 회원 및 캐릭터 온보딩 정보를 제출하고 저장할 수 있습니다.
    • 프로필 이미지, 성별, 직업 정보를 온보딩 과정에서 설정할 수 있습니다.
    • 로그아웃 및 회원 탈퇴가 서버와 소셜 로그인에 연동됩니다.
    • 인증 세션 만료 시 자동으로 로그인 화면으로 이동합니다.
  • 버그 수정

    • 인증 토큰 만료 후 잘못된 자동 로그인을 방지합니다.
    • 온보딩, 로그아웃, 회원 탈퇴의 중복 요청을 차단합니다.
    • 로그아웃 및 탈퇴 처리 결과와 오류 메시지를 더욱 명확하게 표시합니다.
    • 문의 및 티켓 메뉴에서 준비 중 안내를 제공합니다.

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

Gemini API 응답을 읽지 못했습니다.

{
  "error": {
    "code": 503,
    "message": "This model is currently experiencing high demand. Spikes in demand are usually temporary. Please try again later.",
    "status": "UNAVAILABLE"
  }
}

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bca7bf28-3847-4d3c-a33a-db517cbca91d

📥 Commits

Reviewing files that changed from the base of the PR and between 3e7c8d3 and dea3a70.

📒 Files selected for processing (2)
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • core/network/src/main/java/kr/co/call/network/di/NetworkModule.kt

📝 Walkthrough

Walkthrough

온보딩 입력과 제출 API를 추가했습니다. 토큰 저장소와 인증 만료 처리를 변경했습니다. 로그아웃·회원 탈퇴와 Kakao 로그아웃을 연결했습니다. 온보딩과 마이페이지의 로딩, 메시지, 로그인 이동 처리를 갱신했습니다.

Changes

온보딩 및 인증 흐름

Layer / File(s) Summary
온보딩 계약과 API 연동
core/domain/..., core/network/..., core/data/...
회원·캐릭터 입력 모델, 요청 DTO, API, 저장소 구현과 DI 바인딩을 추가했습니다.
온보딩 상태와 제출 UI
feature/onboarding/impl/...
직업 enum, 성별·이미지 상태, 제출 검증, 완료·메시지 사이드 이펙트와 로딩 차단을 추가했습니다.
계정 상태와 로그인 판정
core/datastore/..., core/network/..., feature/login/..., app/...
DataStore 직접 접근을 적용했습니다. 토큰 갱신 실패를 세션 만료 이벤트와 로그인 화면 이동으로 연결했습니다.
마이페이지 인증 동작
core/domain/..., core/data/..., feature/mypage/...
로그아웃·회원 탈퇴 API, 토큰 삭제, Kakao 로그아웃, 오류 메시지, 로딩 상태와 로그인 이동을 연결했습니다.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: ✨ Feature

Sequence Diagram(s)

sequenceDiagram
  participant TokenAuthenticator
  participant AuthSessionManager
  participant AppViewModel
  participant AppScreen
  TokenAuthenticator->>AuthSessionManager: notifySessionExpired()
  AuthSessionManager-->>AppViewModel: sessionExpired
  AppViewModel-->>AppScreen: NavigateToLogin
  AppScreen->>AppScreen: replace back stack with LoginNavKey
Loading
sequenceDiagram
  participant Onboarding5Screen
  participant OnboardingViewModel
  participant OnboardingRepositoryImpl
  participant MyPageApi
  participant AICharacterApi
  Onboarding5Screen->>OnboardingViewModel: submitOnboarding(preferTime)
  OnboardingViewModel->>OnboardingRepositoryImpl: submit onboarding inputs
  OnboardingRepositoryImpl->>MyPageApi: updateMember(request)
  OnboardingRepositoryImpl->>AICharacterApi: createCharacter(request)
  AICharacterApi-->>OnboardingRepositoryImpl: ApiResponse
  OnboardingRepositoryImpl-->>OnboardingViewModel: Result<Unit>
Loading
🚥 Pre-merge checks | ✅ 2 | ❌ 5

❌ Failed checks (5 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning #56은 약관 API 연결을 요구하지만, 변경 사항은 로그아웃·회원 탈퇴·온보딩·인증 세션을 구현하므로 이슈 목표를 충족하지 않습니다. 약관 API를 구현하거나, 현재 변경 사항에 맞는 이슈를 연결하고 #56과 분리하십시오.
Out of Scope Changes check ⚠️ Warning 약관 API 이슈 범위를 벗어나 로그아웃·회원 탈퇴·온보딩·토큰 저장소·세션 만료 처리를 함께 변경했습니다. 약관 API 변경만 남기고 나머지 변경은 별도 이슈와 PR로 분리하십시오.
Docstring Coverage ⚠️ Warning Docstring coverage is 12.24% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Ui 변경 시 스크린샷 첨부 확인 ⚠️ Warning 변경 목록에 MyPageScreen.kt와 Onboarding 화면 등 Compose UI 파일이 포함되지만, PR 설명에 스크린샷 또는 화면 녹화 첨부가 없습니다. 변경된 화면의 스크린샷 또는 화면 녹화 링크를 PR 설명에 추가하세요.
모듈 의존성 방향 검증 ⚠️ Warning core/domain의 ChatRepository.kt 3행이 금지된 androidx.paging.PagingData를 import합니다. feature/login/impl도 core:data 모듈을 직접 의존합니다. ChatRepository에서 AndroidX Paging 타입을 제거하고 순수 domain 타입으로 추상화하세요. feature/login/impl의 core:data 직접 의존은 제거하고 domain 인터페이스만 사용하세요.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 Conventional Commits 형식의 feat: 접두사를 사용하며, 로그아웃·회원 탈퇴 API 변경을 명확히 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/56-login-api

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (2)
feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt (1)

57-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

사이드 이펙트 수집을 LaunchedEffect로 범위 지정하세요.

현재 collectSideEffect를 Composable 본문에서 직접 호출합니다. 프로젝트 규칙에 맞게 LaunchedEffect 내부에서 ViewModel의 side-effect Flow를 collect하세요. collectAsState()는 이미 lifecycle-aware 수집을 제공하므로 변경하지 마세요.

🤖 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 `@feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt` at
line 57, MyPageScreen의 collectSideEffect 호출을 Composable 본문에서 제거하고 LaunchedEffect
범위 안에서 ViewModel의 side-effect Flow를 수집하도록 변경하세요. 기존 collectAsState() 호출은
lifecycle-aware 수집을 위해 그대로 유지하세요.

Sources: Path instructions, Learnings

feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt (1)

70-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

로그아웃과 탈퇴를 UseCase를 통해 호출하세요.

MyPageViewModelMyPageRepository를 직접 호출합니다. 새 인증 흐름은 중복 방지, 상태 전이, 오류 처리, Kakao 로그아웃을 함께 조정합니다.

LogoutUseCaseDeleteAccountUseCasecore/domain에 두고 저장소 작업을 위임하세요. ViewModel은 UI 상태와 side effect 조정만 담당해야 합니다.

As per path instructions, **/*ViewModel.kt의 비즈니스 로직은 core:domain UseCase 호출로 위임해야 합니다.

Also applies to: 98-104

🤖 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
`@feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt`
around lines 70 - 76, MyPageViewModel의 로그아웃 및 회원 탈퇴 비즈니스 로직을 직접
MyPageRepository에서 호출하지 않도록 변경하세요. core:domain의 LogoutUseCase와
DeleteAccountUseCase를 추가해 저장소 작업, Kakao 로그아웃, 상태 전이 및 오류 처리를 위임하고,
MyPageViewModel은 해당 UseCase를 호출한 뒤 UI 상태와 MyPageSideEffect만 조정하도록 수정하세요.

Source: Path instructions

🤖 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
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt`:
- Around line 50-55: Update the login flow around
accountStateDataStore.shouldForceOnboarding() and saveTokens() so the account
state is read successfully before persisting tokens. Move saveTokens() after all
required reads complete, while preserving the existing LoginToken
needsOnboarding calculation and ensuring a read failure returns login failure
without leaving authentication tokens stored.

In
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt`:
- Around line 43-51: In the deleteAccount function, separate the withdrawal
completion state management from token cleanup to handle partial failures. After
myPageApi.deleteAccount() succeeds, call accountStateDataStore.markWithdrawn()
to record the withdrawal state regardless of subsequent token deletion. Move
tokenDataStore.clearTokens() to execute as an independent non-blocking operation
that catches and logs its own failures without propagating them to the overall
result, since token deletion should not prevent the withdrawal from being marked
complete or cancel the deletion operation.

In
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt`:
- Around line 26-67: Update submitOnboarding so the member update and character
creation are atomic, or make createCharacter idempotent by adding an idempotency
key or upsert contract. Ensure retries after updateMember succeeds do not create
duplicate characters and can safely resume the onboarding flow before
clearForceOnboarding() is called.

In
`@core/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.kt`:
- Around line 26-28: Remove the runBlocking { tokenDataStore.getAccessToken() }
call from the AuthInterceptor fallback logic. The interceptor should only read
from memory cache using getCachedAccessToken() and avoid any synchronous
DataStore I/O operations that would block the OkHttp request thread. Instead,
move DataStore reads and token refresh operations to a dedicated token refresh
path that executes before requests are made, or implement explicit timeout and
failure policies at a higher level outside the interceptor itself.

In
`@feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt`:
- Around line 72-76: MyPageViewModel의 KakaoLogoutManager.logout() 실패를 로그만 남긴 채
NavigateToLogin으로 진행하지 않도록 수정하세요. 로그아웃 실패 시 재시도 가능한 사용자 오류로 처리해 로그인 화면 이동을
보류하거나, 이동 전에 카카오 SDK 세션을 별도로 종료해 잔존 세션이 자동 로그인을 유발하지 않도록 하세요.

In
`@feature/onboarding/impl/src/main/java/kr/co/call/impl/component/TemporaryProfileImageData.kt`:
- Around line 8-13: Update the femaleImages definitions alongside the maleImages
list in the temporary profile image data so every selectable female
ProfileImageOption receives a valid imageUrl instead of an empty string. Reuse
the appropriate existing profile image URL constant or provide the corresponding
female image URL while preserving the existing option IDs and list structure.

In
`@feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding1Screen.kt`:
- Around line 183-190: PR 설명에 화면 변경 증빙을 추가하세요. Onboarding1Screen.kt 183-190에서는
회원 직업 및 프로필 선택 동작을, Onboarding2Screen.kt 184-191에서는 AI 직업 선택과 다음 단계 활성화를,
Onboarding6Screen.kt 98-100에서는 제출 중 클릭 차단 동작을 보여주는 스크린샷 또는 화면 녹화를 각각 첨부하세요.

In
`@feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt`:
- Around line 184-191: Update the job-selection flow around selectedJob and
CharacterJob so the displayed options exactly match CharacterJob.label values.
Either replace the "기타" option with the existing "무직" label or add a
corresponding OTHER("기타") enum entry, ensuring selecting the option assigns a
non-null CharacterJob and allows canMoveNext to proceed.

In
`@feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt`:
- Around line 118-205: Extract the validation, OnboardingSubmission
construction, trait priority assignment, and
onboardingRepository.submitOnboarding call from submitOnboarding into a
core-domain SubmitOnboardingUseCase. Update the ViewModel’s submitOnboarding
method to pass the required state inputs and call the use case, retaining only
UI state handling through reduce and postSideEffect, including loading, success,
cancellation, and error presentation.

---

Nitpick comments:
In `@feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt`:
- Line 57: MyPageScreen의 collectSideEffect 호출을 Composable 본문에서 제거하고
LaunchedEffect 범위 안에서 ViewModel의 side-effect Flow를 수집하도록 변경하세요. 기존
collectAsState() 호출은 lifecycle-aware 수집을 위해 그대로 유지하세요.

In
`@feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt`:
- Around line 70-76: MyPageViewModel의 로그아웃 및 회원 탈퇴 비즈니스 로직을 직접
MyPageRepository에서 호출하지 않도록 변경하세요. core:domain의 LogoutUseCase와
DeleteAccountUseCase를 추가해 저장소 작업, Kakao 로그아웃, 상태 전이 및 오류 처리를 위임하고,
MyPageViewModel은 해당 UseCase를 호출한 뒤 UI 상태와 MyPageSideEffect만 조정하도록 수정하세요.
🪄 Autofix (Beta)

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 336f2df7-334b-4a7d-87b8-a6797ad64073

📥 Commits

Reviewing files that changed from the base of the PR and between ee8b34e and 7b1480e.

📒 Files selected for processing (35)
  • app/src/main/java/kr/co/call/callfromai/AppScreen.kt
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt
  • core/datastore/src/main/java/kr/co/call/datastore/AccountStateDataStore.kt
  • core/domain/src/main/java/kr/co/call/domain/model/onboarding/OnboardingSubmission.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/MyPageRepository.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/OnboardingRepository.kt
  • core/network/build.gradle.kts
  • core/network/src/main/java/kr/co/call/network/api/AICharacterApi.kt
  • core/network/src/main/java/kr/co/call/network/api/MyPageApi.kt
  • core/network/src/main/java/kr/co/call/network/di/NetworkModule.kt
  • core/network/src/main/java/kr/co/call/network/dto/onboarding/CreateCharacterRequestDto.kt
  • core/network/src/main/java/kr/co/call/network/dto/onboarding/UpdateMemberRequestDto.kt
  • core/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.kt
  • core/network/src/main/java/kr/co/call/network/util/SafeApiCall.kt
  • feature/mypage/impl/build.gradle.kts
  • feature/mypage/impl/src/main/java/kr/co/call/impl/auth/KakaoLogoutManager.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/entry/MyPageEntryBuilder.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageSideEffect.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageState.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/component/TemporaryProfileImageData.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/entry/OnboardingEntryBuilder.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding1Screen.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding6Screen.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingSideEffect.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingUiState.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/model/Job.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/state/Onboarding1State.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/state/Onboarding2State.kt

Comment on lines +50 to +55
val forceOnboarding=accountStateDataStore.shouldForceOnboarding()

LoginToken(
accessToken = result.accessToken,
refreshToken = result.refreshToken,
needsOnboarding = result.needsOnboarding,
needsOnboarding = result.needsOnboarding || forceOnboarding,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
git ls-files | rg '(^|/)LoginRepositoryImpl\.kt$|AccountStateDataStore'

echo "== inspect LoginRepositoryImpl outline/size =="
FILE="core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt"
if [ -f "$FILE" ]; then
  wc -l "$FILE"
  ast-grep outline "$FILE" --view compact || true
  echo "== relevant lines =="
  cat -n "$FILE" | sed -n '1,120p'
fi

echo "== inspect AccountStateDataStore candidates =="
for f in $(git ls-files | rg 'AccountStateDataStore|accountStateDataStore'); do
  echo "--- $f"
  wc -l "$f"
  ast-grep outline "$f" --view compact || true
  cat -n "$f" | sed -n '1,180p'
done

echo "== search loginWithKakao/saveTokens usages =="
rg -n "loginWithKakao|saveTokens|shouldForceOnboarding|save.*Token|TokenRepository|needsOnboarding" core/data core/domain || true

Repository: call-from-ai/CallFromAi_Android

Length of output: 5550


토큰 저장 전에 계정 상태를 읽으세요.

shouldForceOnboarding() 실패 시 runCatching은 로그인 실패를 반환하지만, Line 45의 saveTokens()는 이미 성공합니다. 모든 읽기가 성공한 뒤에는 저장하도록 순서를 바꾸어 실패 시 인증 토큰이 남지 않게 하세요.

🤖 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
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt`
around lines 50 - 55, Update the login flow around
accountStateDataStore.shouldForceOnboarding() and saveTokens() so the account
state is read successfully before persisting tokens. Move saveTokens() after all
required reads complete, while preserving the existing LoginToken
needsOnboarding calculation and ensuring a read failure returns login failure
without leaving authentication tokens stored.

Comment on lines +43 to +51
override suspend fun deleteAccount(): Result<Unit> =
runRepositoryCatching {
//토큰이 남아 있을 때 탈퇴 api를 먼저 호출
safeApiCallUnit(errorResponseParser){
myPageApi.deleteAccount()
}
//서버 탈퇴 성공 후 기기의 토큰 삭제
tokenDataStore.clearTokens()
accountStateDataStore.markWithdrawn()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# DataStore 예외 전파와 탈퇴 플래그 소비 위치를 확인합니다.
fd -t f -a '^(TokenDataStore|AccountStateDataStore|LoginRepositoryImpl)\.kt$' . |
while IFS= read -r file; do
  echo "===== $file ====="
  rg -n -C 8 'clearTokens|markWithdrawn|withdrawn|onboarding|edit\(' "$file"
done

Repository: call-from-ai/CallFromAi_Android

Length of output: 278


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Changed/stat:"
git diff --stat || true

echo
echo "Find relevant files:"
fd -t f -a 'MyPageRepositoryImpl\.kt|TokenDataStore\.kt|AccountStateDataStore\.kt|LoginRepositoryImpl\.kt|MyPageRepository\.kt' . || true

echo
echo "Search symbols:"
rg -n 'deleteAccount|clearTokens|markWithdrawn|withdrawn|onboarding|runRepositoryCatching|safeApiCallUnit' .

Repository: call-from-ai/CallFromAi_Android

Length of output: 11637


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "===== MyPageRepositoryImpl.kt ====="
cat -n core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt | sed -n '1,90p'

echo
echo "===== safeApiCall utilities ====="
cat -n core/network/src/main/java/kr/co/call/network/util/SafeApiCall.kt | sed -n '1,140p'
cat -n core/data/src/main/java/kr/co/call/data/util/SafeApiResult.kt | sed -n '1,120p'

echo
echo "===== DataStore implementations ====="
cat -n core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt | sed -n '1,230p'
cat -n core/datastore/src/main/java/kr/co/call/datastore/AccountStateDataStore.kt | sed -n '1,80p'

echo
echo "===== Onboarding and MyPage consumers ====="
cat -n core/domain/src/main/java/kr/co/call/domain/repository/OnboardingRepository.kt
cat -n feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt | sed -n '1,230p'
cat -n feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt | sed -n '230,360p'
cat -n feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt | sed -n '1,140p'

Repository: call-from-ai/CallFromAi_Android

Length of output: 27825


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "===== Login auth/initialization consumers for AccountStateDataStore ====="
rg -n -C 10 'AccountStateDataStore|Account.*State|shouldForceOnboarding|force_onboarding|forceOnboarding|tokenDataStore|getAccessToken|login|LoginRepositoryImpl|KakaoLogoutManager|onboardingEntry' app core feature -g '*.kt'

echo
echo "===== LoginRepositoryImpl.kt if present ====="
if [ -f core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt ]; then
  cat -n core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt | sed -n '1,220p'
else
  fd -t f -a 'LoginRepository.*\.kt' . | while read -r f; do echo "===== $f ====="; cat -n "$f" | sed -n '1,220p'; done
fi

Repository: call-from-ai/CallFromAi_Android

Length of output: 50390


탈퇴의 부분 실패 처리를 분리하세요.

myPageApi.deleteAccount() 성공 후 tokenDataStore.clearTokens()가 실패하면 markWithdrawn()이 실행되지 않습니다. MyPageViewModel도 이후 카카오 로그아웃 실패를 기록만 하는 것과 달리 전체 탈퇴 실패로 반환합니다. 토큰 삭제 실패는 3rd-party 로그인 상태를 유지하는 비동기 처리로 줄이고, 탈퇴 완료 상태 저장은 토큰 정리와 분리하세요. 카카오토큰/출석 카드 실패도 실패로 반환하면 탈퇴가 취소 가능합니다.

🤖 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
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt`
around lines 43 - 51, In the deleteAccount function, separate the withdrawal
completion state management from token cleanup to handle partial failures. After
myPageApi.deleteAccount() succeeds, call accountStateDataStore.markWithdrawn()
to record the withdrawal state regardless of subsequent token deletion. Move
tokenDataStore.clearTokens() to execute as an independent non-blocking operation
that catches and logs its own failures without propagating them to the overall
result, since token deletion should not prevent the withdrawal from being marked
complete or cancel the deletion operation.

Comment on lines +26 to +67
runRepositoryCatching {
// 1. 사용자 정보 저장
safeApiCallUnit(errorResponseParser) {
myPageApi.updateMember(
request = UpdateMemberRequestDto(
lastName = submission.member.lastName,
firstName = submission.member.firstName,
imageUrl = submission.member.imageUrl,
gender = submission.member.gender,
birth = submission.member.birth,
mbti = submission.member.mbti,
job = submission.member.job,
),
)
}

// 2. 캐릭터 생성
safeApiCallUnit(errorResponseParser) {
aiCharacterApi.createCharacter(
request = CreateCharacterRequestDto(
lastName = submission.character.lastName,
firstName = submission.character.firstName,
gender = submission.character.gender,
age = submission.character.age,
job = submission.character.job,
imageUrl = submission.character.imageUrl,
spiceLevel = submission.character.spiceLevel,
preferTime = submission.character.preferTime,
mbti = submission.character.mbti,
speechStyle = submission.character.speechStyle,
relationshipStage =
submission.character.relationshipStage,
traits = submission.character.traits.map { trait ->
CharacterTraitRequestDto(
trait = trait.trait,
priority = trait.priority,
)
},
),
)
}
accountStateDataStore.clearForceOnboarding()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# 온보딩 제출과 캐릭터 생성의 멱등성 또는 트랜잭션 계약을 확인합니다.
rg -n -C 5 \
  -g '*.kt' -g '*.yaml' -g '*.yml' -g '*.json' -g '*.md' \
  'submitOnboarding|createCharacter|POST\("characters"\)|idempoten|onboarding' . || true

Repository: call-from-ai/CallFromAi_Android

Length of output: 48904


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== OnboardingRepositoryImpl =="
cat -n core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt

echo
echo "== OnboardingViewModel submitOnboarding branch =="
sed -n '118,195p' feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt | cat -n

echo
echo "== Force onboarding access =="
rg -n -C 4 'setForceOnboarding|clearForceOnboarding|FORCE_ONBOARDING|forceOnboarding' core data feature app | sed -n '1,220p' || true

echo
echo "== API interfaces around character/member requests =="
cat -n core/network/src/main/java/kr/co/call/network/api/AICharacterApi.kt
echo "---"
cat -n core/network/src/main/java/kr/co/call/network/api/MyPageApi.kt
echo "---"
cat -n core/network/src/main/java/kr/co/call/network/dto/onboarding/CreateCharacterRequestDto.kt
echo "---"
cat -n core/network/src/main/java/kr/co/call/network/dto/onboarding/UpdateMemberRequestDto.kt

Repository: call-from-ai/CallFromAi_Android

Length of output: 13884


온보딩 제출을 원자적으로 처리하거나 캐릭터 생성을 멱등하게 만드세요.

submitOnboarding()updateMember() 이후 createCharacter()가 실패하면 clearForceOnboarding()을 호출하지 않아 온보딩 입력값이 저장되고 재시도에서도 POST("characters")를 다시 보내는 경로가 열립니다. 서버에서 온보딩 제출을 하나로 처리하거나, 캐릭터 생성 요청에 idempotency key 또는 upsert 계약을 추가해 이미 완료된 단계가 안전하게 재개되도록 해야 합니다.

🤖 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
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt`
around lines 26 - 67, Update submitOnboarding so the member update and character
creation are atomic, or make createCharacter idempotent by adding an idempotency
key or upsert contract. Ensure retries after updateMember succeeds do not create
duplicate characters and can safely resume the onboarding flow before
clearForceOnboarding() is called.

Comment on lines +26 to +28
?: runBlocking{
tokenDataStore.getAccessToken()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate relevant files =="
fd -a 'AuthInterceptor\.kt|TokenDataStore' . | sed 's#^\./##' || true

echo "== inspect AuthInterceptor =="
for f in $(fd 'AuthInterceptor\.kt' .); do
  echo "--- $f ($(wc -l < "$f") lines) ---"
  cat -n "$f"
done

echo "== inspect TokenDataStore candidates =="
for f in $(fd 'TokenDataStore' .); do
  echo "--- $f ($(wc -l < "$f") lines) ---"
  cat -n "$f"
done

echo "== search for interceptor/cache usages =="
rg -n "AuthInterceptor|runBlocking|DataStore|getAccessToken|tokenCache|cache" -S core/network || true

Repository: call-from-ai/CallFromAi_Android

Length of output: 11391


interceptor에서 DataStore I/O를 동기 대기하지 마세요.

AuthInterceptorgetCachedAccessToken() 실패 시 runBlocking { getAccessToken() }을 호출합니다. getAccessToken()safeData.first()까지 동기적으로 대기하므로, DataStore 파일 읽기가 지연되면 OkHttp 요청 스레드를 블로킹합니다. interceptor는 메모리 캐시만 읽도록 만들고, 캐시 미스 처리를 토큰 재부팅 경로에서 실행하거나 명시적 타임아웃과 실패 정책을 적용하세요.

🤖 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 `@core/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.kt`
around lines 26 - 28, Remove the runBlocking { tokenDataStore.getAccessToken() }
call from the AuthInterceptor fallback logic. The interceptor should only read
from memory cache using getCachedAccessToken() and avoid any synchronous
DataStore I/O operations that would block the OkHttp request thread. Instead,
move DataStore reads and token refresh operations to a dedicated token refresh
path that executes before requests are made, or implement explicit timeout and
failure policies at a higher level outside the interceptor itself.

Source: Path instructions

Comment thread feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt Outdated
Comment on lines +8 to +13
const val DEFAULT_PROFILE_IMAGE_URL =
"https://callfromai-images.s3.ap-northeast-2.amazonaws.com/male_1.png"
val maleImages = List(5) { index ->
ProfileImageOption(
id = "temporary_male_${index + 1}",
imageUrl = "",
imageUrl = DEFAULT_PROFILE_IMAGE_URL,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

여성 프로필 이미지에도 제출 가능한 URL을 제공하세요.

Line 13은 남성 옵션에만 URL을 설정합니다. femaleImages는 빈 imageUrl을 유지합니다. 여성 이미지를 선택하면 Onboarding1Screen 또는 Onboarding2Screen이 빈 문자열을 전달하고, OnboardingViewModel이 이를 API 요청에 포함합니다. 모든 선택 가능한 여성 옵션에 유효한 URL을 지정하세요.

🤖 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
`@feature/onboarding/impl/src/main/java/kr/co/call/impl/component/TemporaryProfileImageData.kt`
around lines 8 - 13, Update the femaleImages definitions alongside the
maleImages list in the temporary profile image data so every selectable female
ProfileImageOption receives a valid imageUrl instead of an empty string. Reuse
the appropriate existing profile image URL constant or provide the corresponding
female image URL while preserving the existing option IDs and list structure.

Comment on lines +183 to +190
selectedOption = selectedJob?.label.orEmpty(),
placeholder = "직업을 선택해주세요",
options = listOf("대학생", "직장인", "기타",),
onOptionSelected = { job = it },
onOptionSelected = {selectedLabel ->
selectedJob=MemberJob.entries.firstOrNull(){job->
job.label == selectedLabel
}
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

화면 변경 증빙을 PR 설명에 추가하세요.

제공된 PR 설명에는 스크린샷 또는 화면 녹화가 없습니다.

  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding1Screen.kt#L183-L190: 회원 직업과 프로필 선택 동작을 보여주세요.
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt#L184-L191: AI 직업 선택과 다음 단계 활성화 동작을 보여주세요.
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding6Screen.kt#L98-L100: 제출 중 클릭 차단 동작을 보여주세요.

As per coding guidelines, “Jetpack Compose 화면 파일이 변경된 경우, PR 설명에 스크린샷 또는 화면 녹화가 첨부되었는지 확인하고 없으면 경고한다.”

📍 Affects 3 files
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding1Screen.kt#L183-L190 (this comment)
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt#L184-L191
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding6Screen.kt#L98-L100
🤖 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
`@feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding1Screen.kt`
around lines 183 - 190, PR 설명에 화면 변경 증빙을 추가하세요. Onboarding1Screen.kt 183-190에서는
회원 직업 및 프로필 선택 동작을, Onboarding2Screen.kt 184-191에서는 AI 직업 선택과 다음 단계 활성화를,
Onboarding6Screen.kt 98-100에서는 제출 중 클릭 차단 동작을 보여주는 스크린샷 또는 화면 녹화를 각각 첨부하세요.

Source: Coding guidelines

Comment thread feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt Outdated
Comment on lines +118 to +205
fun submitOnboarding(
callNow: Boolean,
) = intent {
if (state.submitStatus == LoadStatus.Loading) return@intent

val age = state.aiAge.toIntOrNull()
val speechStyle = state.speechStyle
val relationship = state.relationship
val preferTime = state.preferTime

if (
age == null ||
speechStyle == null ||
relationship == null ||
preferTime == null ||
state.traits.isEmpty()
) {
postSideEffect(
OnboardingSideEffect.ShowMessage(
message = "온보딩 정보를 확인해주세요.",
),
)
return@intent
}

reduce {
state.copy(submitStatus = LoadStatus.Loading)
}

val submission = OnboardingSubmission(
member = MemberOnboardingInput(
lastName = state.userLastName,
firstName = state.userFirstName,
imageUrl = state.userImageUrl,
gender = state.userGender,
birth = state.userBirthday.toString(),
mbti = state.userMbti,
job = state.userJob,
),
character = CharacterOnboardingInput(
lastName = state.aiLastName,
firstName = state.aiFirstName,
gender = state.aiGender,
age = age,
job = state.aiJob,
imageUrl = state.aiImageUrl,
spiceLevel = state.temperature,
preferTime = preferTime.name,
mbti = state.aiMbti,
speechStyle = speechStyle.name,
relationshipStage = relationship.name,
traits = state.traits.mapIndexed { index, trait ->
CharacterTraitInput(
trait = trait.keyword,
priority = index + 1,
)
},
),
)

onboardingRepository.submitOnboarding(submission)
.onSuccess {
reduce {
state.copy(submitStatus = LoadStatus.Idle)
}

postSideEffect(
OnboardingSideEffect.OnboardingCompleted(
callNow = callNow,
),
)
}
.onFailure { error ->
if (error is CancellationException) throw error

reduce {
state.copy(submitStatus = LoadStatus.Idle)
}

postSideEffect(
OnboardingSideEffect.ShowMessage(
message = error.toUserMessage(
default = "온보딩 저장에 실패했습니다.",
),
),
)
}
}

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

온보딩 제출 로직을 UseCase로 위임하세요.

Line 118부터 Line 205까지의 ViewModel이 입력 검증, 제출 모델 조립, 우선순위 생성, Repository 호출을 모두 수행합니다. SubmitOnboardingUseCase로 도메인 제출 규칙과 저장을 이동하세요. ViewModel은 UI 상태 변환, reduce, postSideEffect만 처리하세요.

As per path instructions, “비즈니스 로직이 UseCase(core:domain) 호출로 위임되고 ViewModel에 과도하게 몰려있지 않은지 확인.”

🤖 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
`@feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt`
around lines 118 - 205, Extract the validation, OnboardingSubmission
construction, trait priority assignment, and
onboardingRepository.submitOnboarding call from submitOnboarding into a
core-domain SubmitOnboardingUseCase. Update the ViewModel’s submitOnboarding
method to pass the required state inputs and call the use case, retaining only
UI state handling through reduce and postSideEffect, including loading, success,
cancellation, and error presentation.

Source: Path instructions

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

무시

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dada4679, 알겠습니다. 이 리팩터링 제안은 무시하겠습니다.

ʕ •́ᴥ•̀ʔ

You are interacting with an AI system.

@codebidoof codebidoof left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디벨롭 풀 받구 아원이가 개선해놓은 safeApiCall 쓰면 될 것 같아!

Comment on lines 35 to 53
override suspend fun logout():Result<Unit> =
runRepositoryCatching {
safeEmptyApiCall(errorResponseParser){
myPageApi.logout()
}
//서버 로그아웃 성공 후 기기 토큰 삭제
tokenDataStore.clearTokens()
}
override suspend fun deleteAccount(): Result<Unit> =
runRepositoryCatching {
//토큰이 남아 있을 때 탈퇴 api를 먼저 호출
safeApiCallUnit(errorResponseParser){
myPageApi.deleteAccount()
}
//서버 탈퇴 성공 후 기기의 토큰 삭제
tokenDataStore.clearTokens()
accountStateDataStore.markWithdrawn()
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 이제 아원이가 수정을 해서 runRepositoryCatching 없이 safeApiCall류의 함수만 호출해도 될 거야!!

Comment on lines +11 to +33
@Singleton
class AccountStateDataStore @Inject constructor(
private val dataStore: DataStore<Preferences>,
){
suspend fun markWithdrawn(){
dataStore.edit {preferences->
preferences[FORCE_ONBOARDING]=true
}
}
suspend fun shouldForceOnboarding(): Boolean{
return dataStore.data.first()[FORCE_ONBOARDING] ?:false
}
suspend fun clearForceOnboarding() {
dataStore.edit { preferences ->
preferences.remove(FORCE_ONBOARDING)
}
}

private companion object {
val FORCE_ONBOARDING =
booleanPreferencesKey("force_onboarding_after_withdrawal")
}
} No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아마 이거 정확한 맥락은 모르겠는데 회원탈퇴 or 로그아웃 후 다시 로그인할 시에 온보딩을 보여지게 하는 것을 강제하기 위함 같은데, needsOnboarding 필드를 로그인시에 백엔드에서 내려주기 때문에 이 로직은 필요 없어 보입니다!

Comment on lines +3 to +36
data class OnboardingSubmission(
val member: MemberOnboardingInput,
val character: CharacterOnboardingInput,
)

data class MemberOnboardingInput(
val lastName: String,
val firstName: String,
val imageUrl: String,
val gender: String,
val birth: String,
val mbti: String,
val job: String,
)

data class CharacterOnboardingInput(
val lastName: String,
val firstName: String,
val gender: String,
val age: Int,
val job: String,
val imageUrl: String,
val spiceLevel: Int,
val preferTime: String,
val mbti: String,
val speechStyle: String,
val relationshipStage: String,
val traits: List<CharacterTraitInput>,
)

data class CharacterTraitInput(
val trait: String,
val priority: Int,
) No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석을 달아주면 리뷰가 편해용

Comment thread core/network/build.gradle.kts Outdated

// retrofit, okhttp
implementation(libs.retrofit)
api(libs.retrofit)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

롤백부탁

Comment on lines +3 to +21
data class CreateCharacterRequestDto(
val lastName: String,
val firstName: String,
val gender: String,
val age: Int,
val job: String,
val imageUrl: String,
val spiceLevel: Int,
val preferTime: String,
val mbti: String,
val speechStyle: String,
val relationshipStage: String,
val traits: List<CharacterTraitRequestDto>
)

data class CharacterTraitRequestDto(
val trait: String,
val priority: Int,
) No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석 부탁드려영

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요! PR 잘 올려주셨습니다. 전체적으로 MVI 패턴과 Clean Architecture 원칙을 잘 적용하고 계셔서 인상 깊습니다. 특히 전역 세션 만료 처리, DataStore 백업 제외, Repository 레이어의 Result 패턴 적용 등 중요한 개선사항들이 눈에 띕니다. 몇 가지 개선할 점과 질문이 있어 코멘트 남깁니다.


1. Kotlin 코드 (일반)

  • core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt
    • TokenDataStore에서 cachedTokens (AtomicReference)와 applicationScope를 제거하고 getTokens()가 직접 dataStore.data.first()를 호출하도록 변경되었습니다.
    • 의견: 이전 코드는 AuthInterceptor가 디스크 I/O를 발생시키지 않도록 메모리 캐싱을 시도한 것으로 보입니다. 현재는 AuthInterceptorTokenAuthenticator 모두 runBlocking { tokenDataStore.getTokens() }를 사용하고 있습니다. DataStore.data.first()는 내부적으로 file.readData()를 호출하므로, 이는 네트워크 스레드에서 블로킹 I/O를 유발할 수 있습니다. DataStoreDispatchers.IO에서 실행되지만, runBlocking은 호출 스레드를 블록하므로 이 부분이 잠재적인 성능 병목이 될 수 있습니다.
    • 제안: AuthInterceptor는 동기적으로 토큰을 제공해야 하므로 StateFlow와 같은 비동기적인 메커니즘을 사용하기 어렵습니다. 현재 방식은 대부분의 경우 문제가 되지 않을 수 있으나, 만약 앱의 인증 흐름이 매우 빈번하거나 I/O 지연이 발생할 경우, 앱 시작 시 토큰을 미리 로드하여 AuthInterceptorStateFlow 형태로 제공하고, AuthInterceptor에서는 StateFlow.value를 사용하는 방안을 고려해볼 수 있습니다. 하지만 이는 TokenAuthenticator의 토큰 갱신 로직과 복잡하게 얽힐 수 있으니 현재로서는 모니터링 대상으로 우선 두고 넘어가도 좋습니다.
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/state/Onboarding1State.kt / Onboarding2State.kt
    • data class 내의 gender, job, mbti, speechStyle, relationshipStage 등의 프로퍼티가 var로 선언되어 있습니다.
    • 제안: data class의 프로퍼티는 불변성(immutability)을 유지하기 위해 val로 선언하는 것이 좋습니다. 상태 변경은 copy() 메서드를 통해 새로운 상태 객체를 생성하여 처리하는 것이 Compose의 Recomposition이나 MVI 상태 관리에도 더 적합합니다. 이 부분들을 val로 변경하고, ViewModel에서 state.copy(...)로 업데이트하는 방식으로 전환해주세요.
  • 하드코딩된 문자열/매직 넘버
    • core/domain/src/main/java/kr/co/call/domain/model/onboarding/OnboardingInput.kt: gender, job, mbti, speechStyle, relationshipStage, trait 등이 String 타입으로 선언되어 있습니다. 이 값들이 미리 정의된 고정된 카테고리라면, MemberJob, CharacterJob, PreferTime처럼 enum class로 정의하여 사용하는 것이 타입 안정성을 높이고 오류를 방지하는 좋은 방법입니다.
    • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt (L183): "대학생", "직장인", "기타"와 같은 직업 옵션이 하드코딩되어 있습니다. CharacterJob.entries.map { it.label }과 같이 enum을 활용하여 목록을 생성하면 재사용성과 유지보수성이 향상됩니다.
    • feature/onboarding/impl/src/main/java/kr/co/call/impl/component/TemporaryProfileImageData.kt (L8): DEFAULT_PROFILE_IMAGE_URL이 하드코딩되어 있습니다. 이 URL이 변경될 가능성이 있거나 환경(개발/운영)에 따라 달라질 수 있다면, buildConfigField나 앱 리소스 문자열로 관리하는 것을 고려해볼 수 있습니다. 현재는 임시 이미지로 보이므로 우선순위는 낮습니다.

2. Jetpack Compose 코드

  • AppScreen.kt (L80-L86), feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt (L51-L69), feature/onboarding/impl/src/main/java/kr/co/call/impl/entry/OnboardingEntryBuilder.kt (L98-L108)
    • viewModel.collectSideEffect { ... }를 사용하여 Orbit의 Side Effect를 Composable에서 올바르게 수집하고 있습니다. 이벤트를 기반으로 한 번의 동작(네비게이션, Toast 표시 등)을 처리하는 데 적절하게 사용되었습니다. Good!
  • feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt (L115, L125)
    • 다이얼로그의 onConfirm, onDismiss 콜백에서 state.authStatus != LoadStatus.Loading을 체크하여 로딩 중 중복 클릭을 방지한 점은 사용자 경험 측면에서 매우 좋습니다. Good!
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding5Screen.kt (L86), Onboarding6Screen.kt (L98, L109)
    • isLoading 파라미터를 받아 버튼 enabled 상태를 제어하는 부분이 좋습니다. 로딩 상태를 UI에 명확히 반영하여 불필요한 사용자 상호작용을 방지합니다. Good!
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt (L123)
    • Column.verticalScroll(rememberScrollState())를 추가하여 내용이 길어질 경우 스크롤이 가능하도록 한 점이 좋습니다. Good!
  • UI 상태를 data class로 표현: MyPageState, OnboardingState 등 UI 상태를 data class로 표현하고 있습니다. LoadStatus를 사용하여 로딩/에러 상태를 관리하는 점도 좋습니다. 다만 위 Kotlin 코드 리뷰에서 언급했듯이, Onboarding1State / Onboarding2State 내의 var 프로퍼티는 val로 변경하는 것이 더 좋습니다.

3. Repository/DataSource 레이어

  • core/data/src/main/java/kr/co/call/data/util/SafeApiResult.kt (New File)
    • runRepositoryCatching 유틸리티 함수를 도입하여 Repository 레이어에서 발생하는 예외를 Result<T>로 래핑하고, CancellationException을 재전파하는 로직은 매우 훌륭합니다. 이로써 비즈니스 로직 및 ViewModel에서 일관된 방식으로 오류를 처리할 수 있게 됩니다. Good!
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt (L34, L44)
    • logout()deleteAccount() 메서드가 Result<Unit>을 반환하도록 변경되고 runRepositoryCatchingsafeApiCallUnit을 사용하여 Retrofit 에러를 일관되게 핸들링하고 있습니다.
    • 특히 deleteAccount()에서 tokenDataStore.clearTokens() 실패 시 CancellationException을 재전파하고 나머지는 경고로 로그만 남기는 처리 (onFailure 내부 Timber.w)는 예외 처리 우선순위를 잘 고려한 좋은 예입니다. Good!
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt (New File)
    • submitOnboarding에서 두 개의 API 호출을 safeApiCallUnit으로 각각 래핑하고, 전체를 runRepositoryCatching으로 감싸는 방식은 복합적인 비즈니스 로직에 대한 에러 핸들링을 명확하게 보여줍니다.
    • 네트워크 응답 DTO (UpdateMemberRequestDto, CreateCharacterRequestDto)와 도메인 모델 (OnboardingInput) 매핑이 분리되어 있습니다. Repository에서 도메인 모델을 받아 네트워크 DTO로 변환하여 API를 호출하는 방식이 올바릅니다. Good!
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt (L13)
    • getMyProfile()은 아직 delay(500)과 하드코딩된 데이터를 사용하는 Mock 구현체입니다. 실제 API 연동 시 위에서 언급된 runRepositoryCatching, safeApiCall 등의 패턴을 적용하여 Result<MyPageProfile>을 반환하도록 수정해야 합니다. (TODO 주석도 제거해야 합니다)

4. ViewModel

  • AppViewModel.kt (L19-L23)
    • init 블록에서 authSessionManager.sessionExpired.collectintent { ... }로 감싸 viewModelScope 내에서 안전하게 Flow를 수집하고 postSideEffect로 전역 네비게이션을 트리거하는 방식이 좋습니다. AuthSessionManager가 Flow를 발행하므로 블로킹 작업이 아니어서 intent 내부에서 직접 collect하는 것이 적절합니다. Good!
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt
    • logout()deleteAccount() 메서드에서 if (state.authStatus == LoadStatus.Loading) return@intent를 사용하여 중복 요청을 방지하는 부분이 좋습니다.
    • 서버 API 호출 (myPageRepository.logout())과 외부 SDK 호출 (kakaoLogoutManager.logout())의 실패를 개별적으로 처리하고 사용자에게 Toast 메시지로 피드백을 주는 로직이 상세하고 robust합니다. ShowMessage Side Effect를 활용하여 UI 종속성을 줄인 것도 좋습니다. Good!
    • deleteAccount()에서 kakaoLogoutManager.logout()의 실패는 Timber.w로 경고만 남기고 전체 deleteAccount 작업의 Result에는 영향을 주지 않도록 한 점은 사용자에게 치명적인 실패(계정 삭제 실패)를 먼저 알리고 부가적인 실패(카카오 로그아웃 실패)는 로그로 추적하는 합리적인 판단입니다. Good!
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt (New File)
    • submitOnboarding에서 if (state.submitStatus == LoadStatus.Loading) return@intent로 중복 호출을 방지하고, reduce { state.copy(submitStatus = LoadStatus.Loading) }으로 로딩 상태를 관리하는 점이 좋습니다.
    • Repository로부터 받은 ResultonSuccessonFailure로 분기하여 각각 OnboardingSubmitted Side Effect와 ShowMessage Side Effect를 발생시키는 방식이 MVI 원칙에 잘 맞습니다. Good!
  • intent 내부에서 상태 변경(reduce)만 수행하는지: 대부분의 handleIntent 메서드와 private 함수들이 reduce 또는 postSideEffect를 사용하여 상태 변경 및 Side Effect를 발행하고 있습니다. 비즈니스 로직과 UI 상태 관리가 잘 분리되어 있습니다. Good!

5. FCM/SSE/실시간 통신 관련 코드

  • 해당 PR에는 직접적인 FCM/SSE/실시간 통신 관련 변경사항은 없습니다.

그 외 잡다한 코멘트

  • No newline at end of file 수정: 여러 파일에서 파일 끝에 새 줄을 추가하여 Lint 경고를 해결한 점 좋습니다.

전반적으로 품질 높은 PR입니다. 위에서 언급한 몇 가지 개선사항들(특히 data classvar -> val 변경과 enum 사용)을 반영해주시면 더 견고하고 유지보수하기 쉬운 코드가 될 것입니다.

수고하셨습니다!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (2)
core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt (1)

172-184: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

clearTokens() 실패 시 세션 만료 알림이 전달되지 않습니다.

Refresh Token까지 만료되어 401을 받은 경우, authSessionManager.notifySessionExpired()tokenDataStore.clearTokens()와 같은 try 블록 안에 있습니다. clearTokens()가 예외를 던지면 catch로 이동해 알림 호출이 실행되지 않습니다. 이 경우 세션은 실질적으로 만료되었는데도 로그인 화면으로 이동하지 않아 사용자가 인증 오류 상태에 머무를 수 있습니다.

🛡️ 제안하는 수정
                 if (reissueResponse.code() == 401) {
                     try {
                         tokenDataStore.clearTokens()
-                        authSessionManager.notifySessionExpired()
                     } catch (error: CancellationException) {
                         throw error
                     } catch (error: Throwable) {
                         Timber.e(
                             error,
                             "만료된 토큰 삭제 실패",
                         )
+                    } finally {
+                        authSessionManager.notifySessionExpired()
                     }
                 }
🤖 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
`@core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt`
around lines 172 - 184, Separate token cleanup from session-expiration
notification in the 401 handling within TokenAuthenticator: ensure
authSessionManager.notifySessionExpired() is invoked even when
tokenDataStore.clearTokens() throws. Preserve CancellationException propagation
and continue logging other cleanup failures without preventing the expiration
notification.
feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt (1)

105-130: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

deleteAccount()에서 카카오 로그아웃 실패가 무시된 채 로그인 화면으로 이동합니다.

logout()은 이미 카카오 로그아웃 실패 시 NavigateToLogin을 막고 오류 메시지를 표시하도록 수정되었습니다. deleteAccount()는 동일한 패턴을 적용하지 않았습니다. kakaoLogoutManager.logout() 실패는 로그로만 남기고 항상 postSideEffect(MyPageSideEffect.NavigateToLogin)을 호출합니다.

계정은 이미 삭제되어 API 재시도 대상은 아니지만, 카카오 SDK 세션이 로컬에 남으면 다음 로그인 시도에서 자동 로그인이 발생해 삭제된 계정 흐름과 불일치할 수 있습니다. KakaoLogoutManagerunlink()와 같은 강제 무효화 수단이 있는지 확인하고, 실패 시 처리를 logout()과 일관되게 맞추세요.

#!/bin/bash
set -euo pipefail
echo "== KakaoLogoutManager 구현 확인 =="
fd -a 'KakaoLogoutManager.kt' --exec cat -n {}
🤖 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
`@feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt`
around lines 105 - 130, Update deleteAccount() to handle
kakaoLogoutManager.logout() failures consistently with the existing logout flow:
do not navigate to login when logout fails, show the appropriate error message,
and use KakaoLogoutManager.unlink() or another available forced invalidation
method if needed. Preserve the successful logout path’s NavigateToLogin side
effect and existing account-deletion failure handling.
🧹 Nitpick comments (1)
core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt (1)

30-46: 🔒 Security & Privacy | 🔵 Trivial

참고: Access/Refresh Token이 평문 Preferences에 저장됩니다.

이번 diff는 함수명만 saveTokens에서 setTokens로 바꾼 것이라 새로운 결함은 아닙니다. 다만 core/datastore 경로 지침은 민감 데이터의 암호화 여부 확인을 요구합니다. 토큰이 평문 stringPreferencesKey로 저장되므로, 백업 제외 설정과 별개로 기기 루팅이나 파일 시스템 접근 시 노출될 수 있습니다. 장기적으로 androidx.security.crypto 등을 이용한 암호화 저장을 검토하세요.

As per path instructions, "토큰/인증 정보 등 민감 데이터를 평문 DataStore에 저장하지 않는지(암호화 여부) 확인."

🤖 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 `@core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt` around
lines 30 - 46, Update setTokens and the corresponding token retrieval flow to
store ACCESS_TOKEN and REFRESH_TOKEN through encrypted storage rather than plain
string Preferences, using the project’s supported security mechanism such as
androidx.security.crypto. Preserve the existing validation and token access
behavior while ensuring both sensitive values are encrypted at rest.

Source: Path instructions

🤖 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 `@app/src/main/res/xml/backup_rules.xml`:
- Around line 13-15: Update the exclusion path in
app/src/main/res/xml/backup_rules.xml lines 13-15 and
app/src/main/res/xml/data_extraction_rules.xml lines 12-13 to
datastore/call_from_ai_token_prefs.preferences_pb, matching the file name
created by TokenDataStore; both sites require the same direct change.

In `@core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt`:
- Around line 25-27: Update getTokens() to catch DataStore read/I/O exceptions
from dataStore.data.first() and return an empty-token result equivalent to
emptyPreferences() when reading fails. Keep the normal toStoredTokens()
conversion unchanged for successful reads, so AuthInterceptor.intercept(),
TokenAuthenticator.authenticate(), and
LandingViewModel.checkAutoLoginAfterSplash() receive a safe fallback instead of
an uncaught exception.

---

Outside diff comments:
In
`@core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt`:
- Around line 172-184: Separate token cleanup from session-expiration
notification in the 401 handling within TokenAuthenticator: ensure
authSessionManager.notifySessionExpired() is invoked even when
tokenDataStore.clearTokens() throws. Preserve CancellationException propagation
and continue logging other cleanup failures without preventing the expiration
notification.

In
`@feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt`:
- Around line 105-130: Update deleteAccount() to handle
kakaoLogoutManager.logout() failures consistently with the existing logout flow:
do not navigate to login when logout fails, show the appropriate error message,
and use KakaoLogoutManager.unlink() or another available forced invalidation
method if needed. Preserve the successful logout path’s NavigateToLogin side
effect and existing account-deletion failure handling.

---

Nitpick comments:
In `@core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt`:
- Around line 30-46: Update setTokens and the corresponding token retrieval flow
to store ACCESS_TOKEN and REFRESH_TOKEN through encrypted storage rather than
plain string Preferences, using the project’s supported security mechanism such
as androidx.security.crypto. Preserve the existing validation and token access
behavior while ensuring both sensitive values are encrypted at rest.
🪄 Autofix (Beta)

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 14ea7d68-b7a2-4a65-9980-da1da44bec7b

📥 Commits

Reviewing files that changed from the base of the PR and between 7b1480e and bd0d5ab.

📒 Files selected for processing (24)
  • app/src/main/java/kr/co/call/callfromai/AppScreen.kt
  • app/src/main/java/kr/co/call/callfromai/AppViewModel.kt
  • app/src/main/java/kr/co/call/callfromai/sideeffect/AppSideEffect.kt
  • app/src/main/res/xml/backup_rules.xml
  • app/src/main/res/xml/data_extraction_rules.xml
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt
  • core/data/src/main/java/kr/co/call/data/util/SafeApiResult.kt
  • core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt
  • core/domain/src/main/java/kr/co/call/domain/model/onboarding/OnboardingInput.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/OnboardingRepository.kt
  • core/network/src/main/java/kr/co/call/network/api/MyPageApi.kt
  • core/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.kt
  • core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt
  • feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LandingViewModel.kt
  • feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/entry/OnboardingEntryBuilder.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding5Screen.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingSideEffect.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/model/Job.kt
💤 Files with no reviewable changes (1)
  • core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
🚧 Files skipped from review as they are similar to previous changes (4)
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/model/Job.kt
  • core/network/src/main/java/kr/co/call/network/api/MyPageApi.kt
  • core/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding2Screen.kt

Comment thread app/src/main/res/xml/backup_rules.xml
Comment on lines +25 to 27
suspend fun getTokens(): StoredTokens {
return dataStore.data.first().toStoredTokens()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "== DataStore 생성/corruptionHandler 설정 확인 =="
rg -n -C5 'preferencesDataStore|DataStoreFactory|PreferenceDataStoreFactory|corruptionHandler' --type=kt
echo "== getTokens 호출처 확인 =="
rg -n -C3 '\.getTokens\(' --type=kt

Repository: call-from-ai/CallFromAi_Android

Length of output: 243


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate TokenDataStore.kt =="
fd -a 'TokenDataStore.kt' . || true

echo "== DataStore creation/settings in Kotlin files =="
rg -n -C5 'preferencesDataStore|DataStoreFactory|PreferenceDataStoreFactory|corruptionHandler|DataStore<|DataStorePreferences' -g '*.kt' . || true

echo "== getTokens callers in Kotlin files =="
rg -n -C3 'getTokens\(' -g '*.kt' . || true

echo "== token-related DataStore files =="
rg -n -C3 'TokenDataStore|StoredTokens|PreferenceDataStoreFactory|corruptionHandler|dataStore\.data' -g '*.kt' . || true

Repository: call-from-ai/CallFromAi_Android

Length of output: 20653


🌐 Web query:

AndroidX DataStore Preferences ReplaceFileCorruptionHandler IOException catch behavior documentation

💡 Result:

In AndroidX DataStore, the ReplaceFileCorruptionHandler is designed to manage scenarios where data corruption (indicated by a CorruptionException) prevents the reading of a file [1][2]. Its behavior regarding exceptions encountered during the recovery process is as follows: If the ReplaceFileCorruptionHandler succeeds in replacing the corrupted data with new data (via its produceNewData logic), the original CorruptionException is considered handled and is swallowed [2][3]. If the ReplaceFileCorruptionHandler encounters an exception while attempting to replace the data, that new exception is added as a suppressed exception to the original CorruptionException, and the original exception is then thrown [2][3]. Additionally, if the handleCorruption function itself throws an exception during its execution, that exception will be propagated to the original call to the DataStore that triggered the corruption handling [1][2]. It is important to note that the handleCorruption function should not interact with any DataStore API, as doing so can result in a deadlock [1][3]. The CorruptionException, which triggers this process, is a subclass of IOException and is specifically thrown by serializers when data cannot be deserialized, rather than for transient IO or permission issues [4].

Citations:


🌐 Web query:

AndroidX DataStore data Flow catches IOException catch returns from flow documentation ReadPreferences

💡 Result:

When reading data from AndroidX DataStore using the data property, the returned Flow can throw an IOException if an error occurs while reading from disk [1]. To handle these errors effectively, official documentation and standard practices recommend using the catch Flow operator before applying map [2]. Within the catch block, you should inspect the exception: if it is an IOException, you can log the error and emit a default value (such as emptyPreferences) to allow the flow to continue [3][2]. If the exception is of any other type, it should typically be re-thrown to avoid masking unexpected issues [2][4]. Recommended implementation pattern: val userPreferencesFlow: Flow = dataStore.data.catch { exception -> if (exception is IOException) { // Log the error and emit a default/empty value emit(emptyPreferences) } else { // Re-throw unexpected exceptions throw exception } }.map { preferences -> // Proceed with mapping to your data model val exampleValue = preferences[EXAMPLE_KEY]?: defaultValue UserPreferences(exampleValue) } This approach ensures that your application remains resilient to transient I/O errors while still exposing critical failures for proper debugging [2]. For cases involving file corruption, you may also consider configuring a CorruptionHandler when initializing your DataStore instance to automatically replace corrupted files [5].

Citations:


🌐 Web query:

DataStore Core ReadPreferences catch IOException and rethrow corruption exception source

💡 Result:

In Android DataStore, a CorruptionException is a specific subclass of IOException used to signal that the data on disk cannot be deserialized [1]. When DataStore encounters this error during a read, it invokes a configured CorruptionHandler [2][3]. If no custom handler is provided, DataStore effectively uses the default ReThrowCorruptionHandler, which does nothing but rethrow the CorruptionException, causing it to propagate to the original caller [4][5]. Developers can customize this behavior by providing a CorruptionHandler during DataStore creation [3]. For example, the ReplaceFileCorruptionHandler is a common implementation that attempts to recover by replacing the corrupted file with new, default data [6][7]. Key points regarding this mechanism include: 1. CorruptionException vs. Generic IOException: CorruptionException should only be thrown by a serializer when data is unparseable or corrupted [8][1]. It should not be thrown for transient IO issues, such as file permissions or disk availability [1]. 2. Propagation: If the handleCorruption function itself throws an exception, that exception is propagated to the original DataStore caller [2][9]. 3. Deadlocks: CorruptionHandlers must not interact with any DataStore API, as doing so can result in a deadlock [4][2][9]. 4. Implementation Details: When using ReplaceFileCorruptionHandler, if the handler successfully replaces the data, the original exception is swallowed; if the handler fails, the new exception is added as a suppressed exception to the original and the original exception is thrown [6][7].

Citations:


🌐 Web query:

DataStore core ReadPreferences catch IOException read exceptions catchCorruption propagate catch IOException source

💡 Result:

In Jetpack DataStore, IOException and its subclass CorruptionException are handled through different mechanisms depending on the cause of the failure [1][2]. General IOException Handling DataStore operations, such as reading data via the flow property or writing via the updateData function, will throw an IOException if a disk-related error occurs [3][4]. Because the read API exposes a Flow, developers are expected to handle these exceptions using standard flow operators, such as the catch operator, directly in their implementation [5]. If a read fails, the exception is propagated to the collector of the flow, and subsequent collection attempts will retry the read [3][4]. Corruption Handling When data cannot be deserialized, the serializer throws a CorruptionException [6][7][2]. This specific type of IOException is managed by a CorruptionHandler defined at the time of DataStore creation [8][9]. 1. CorruptionHandler: This interface defines the handleCorruption(ex: CorruptionException) method, which DataStore calls when corruption is detected [9][10]. 2. Propagation: If the handleCorruption function itself throws an exception, that exception is propagated to the original caller of the DataStore operation [9][11][12]. 3. Recovery: If handleCorruption returns a value (e.g., a default or empty state), DataStore attempts to write that value to disk, potentially recovering from the corruption [9][11][10]. 4. Built-in Handlers: - ReThrowCorruptionHandler: The default handler that simply rethrows the CorruptionException, effectively propagating it to the caller [9][12]. - ReplaceFileCorruptionHandler: Attempts to replace the corrupted file with a new, provided value [11][10]. If this replacement process itself fails, the new exception is added as a suppressed exception to the original CorruptionException, which is then thrown [11]. It is important to note that CorruptionException should only be thrown by serializers when data is unparseable or corrupted [2]. Transient I/O issues or file system permissions errors should not be wrapped in a CorruptionException, as these should remain as generic IOExceptions [7][2]. Additionally, corruption handlers should never interact with DataStore APIs directly to avoid deadlocks [9][11].

Citations:


getTokens()에서 DataStore 예외를 호출 경로에서 처리해 주세요.

corruptionHandler는 파일 취소 값 복구를 위한 별도 경로를 담당하지만, 토큰 저장 파일의 기본 I/O 읽기 실패는 corruptionHandler만으로 잡히지 않습니다. dataStore.data는 Disk 관련 오류 시 호출자로 전파될 수 있습니다.

수신 경로인 AuthInterceptor.intercept(), TokenAuthenticator.authenticate(), LandingViewModel.checkAutoLoginAfterSplash()에서는 실제 예외 처리 없이 호출하기 때문에, 이 경로에서 emptyPreferences() 반환과 같은 처리가 필요합니다.

🤖 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 `@core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt` around
lines 25 - 27, Update getTokens() to catch DataStore read/I/O exceptions from
dataStore.data.first() and return an empty-token result equivalent to
emptyPreferences() when reading fails. Keep the normal toStoredTokens()
conversion unchanged for successful reads, so AuthInterceptor.intercept(),
TokenAuthenticator.authenticate(), and
LandingViewModel.checkAutoLoginAfterSplash() receive a safe fallback instead of
an uncaught exception.

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

Gemini API 응답을 읽지 못했습니다.

{
  "error": {
    "code": 503,
    "message": "This model is currently experiencing high demand. Spikes in demand are usually temporary. Please try again later.",
    "status": "UNAVAILABLE"
  }
}

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요. 시니어 Android 개발자로서 GitHub Pull Request를 리뷰해 드리겠습니다. 전반적으로 코드 품질 향상과 기능 추가를 위한 좋은 변경들이 많이 보입니다. 특히 DataStore 백업 규칙 적용, Orbit MVI 패턴을 사용한 SideEffect 처리, Repository 계층의 견고한 에러 핸들링, 그리고 Compose UI의 로딩 상태 관리 및 타입 안전성 개선 노력이 돋보입니다.

몇 가지 개선할 점과 질문이 있으니 아래 내용을 참고해 주세요.


전반적인 의견 (General Comments)

  1. 새로운 파일에 \ No newline at end of file 제거: 새로 추가된 AppSideEffect.kt, TokenDataStore.kt, MyPageRepository.kt, OnboardingInput.kt, OnboardingRepository.kt, AICharacterApi.kt, MyPageApi.kt, CreateCharacterRequestDto.kt, UpdateMemberRequestDto.kt, AuthInterceptor.kt, TokenAuthenticator.kt, SafeApiCall.kt, LandingViewModel.kt, KakaoLogoutManager.kt, MyPageSideEffect.kt, MyPageState.kt, MyPageViewModel.kt, Onboarding5Screen.kt, Onboarding6Screen.kt 파일들 끝에 \ No newline at end of file 표시가 있습니다. 파일의 마지막에 항상 빈 줄을 추가하는 것이 일반적인 컨벤션이며, 일부 IDE나 툴에서 문제를 일으킬 수 있습니다. .editorconfig 설정을 확인하거나 수동으로 수정해 주세요.

Kotlin & Android Core

app/src/main/java/kr/co/call/callfromai/AppViewModel.kt

  • Coroutine Dispatcher 명시: init 블록 내에서 authSessionManager.sessionExpired.collect를 호출하고 있습니다. intent 블록은 기본적으로 viewModelScope.launch { container.orbit(...)}와 유사하게 동작하며, Orbit의 내부 Dispatcher가 처리하지만, collect는 upstream의 Dispatcher를 따릅니다. sessionExpired Flow가 어떤 Dispatcher에서 방출되는지에 따라 주의가 필요합니다. 만약 I/O 작업을 포함한다면 flowOn(Dispatchers.IO) 등을 통해 명시적으로 Dispatcher를 지정하는 것을 고려해 보세요.
    init {
        intent {
            // authSessionManager.sessionExpired 의 collect 호출은 viewModelScope에서 진행되지만,
            // sessionExpired 플로우의 업스트림이 어떤 Dispatcher에서 실행되는지 확인이 필요합니다.
            // 만약 I/O 작업이 포함된다면, flowOn(Dispatchers.IO) 등을 고려할 수 있습니다.
            authSessionManager.sessionExpired
                .collect {
                    postSideEffect(AppSideEffect.NavigateToLogin)
                }
        }
    }

app/src/main/res/xml/backup_rules.xml & app/src/main/res/xml/data_extraction_rules.xml

  • 인증 정보 백업 제외: Refresh Token 등 인증 정보가 백업에 포함되지 않도록 exclude 규칙을 추가한 것은 매우 훌륭합니다. 앱 삭제 후 재설치 시 자동 로그인되는 보안 및 UX 문제를 효과적으로 방지할 수 있습니다.

core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt

  • deleteAccountonSuccess 블록 내 try-catch: onSuccess 블록 내에서 CancellationExceptiontry-catch하고 다시 throw하는 패턴은 불필요합니다. Result.onSuccess 람다 자체가 suspend context에서 실행되므로, CancellationException은 상위 코루틴으로 자연스럽게 전파됩니다. Throwable에 대한 Timber.w 로깅은 좋습니다.
    // 변경 전:
    // ...
    // }.onSuccess {
    //     try {
    //         tokenDataStore.clearTokens()
    //     } catch (error: CancellationException) {
    //         throw error
    //     } catch (error: Throwable) {
    //         Timber.w(error, "회원 탈퇴 후 토큰 삭제 실패")
    //     }
    // }
    
    // 변경 후:
    }.onSuccess {
        tokenDataStore.clearTokens()
    }.onFailure { error ->
        if (error is CancellationException) throw error // CancellationException은 자연스럽게 전파됩니다.
        Timber.w(error, "회원 탈퇴 후 토큰 삭제 실패") // 다른 Throwable만 여기서 처리
    }

core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt

  • 성능 관련 우려 (Performance Concern): 이 PR에서 TokenDataStore의 내부 메모리 캐싱(AtomicReference, applicationScope.launch를 통한 safeData flow collect) 로직이 제거되고, 모든 토큰 접근이 dataStore.data.first()를 통해 이루어지도록 변경되었습니다.

    • 문제점: AuthInterceptorTokenAuthenticator 등 HTTP 요청 시 토큰이 필요한 부분에서 runBlocking { tokenDataStore.getTokens().accessToken }와 같이 runBlocking을 사용하여 DataStore를 직접 읽는 것은 심각한 성능 저하를 초래할 수 있습니다. DataStore.data.first()는 디스크 I/O를 포함하며, 이는 모든 네트워크 요청마다 동기적으로 발생하여 애플리케이션의 응답성을 떨어뜨릴 수 있습니다.
    • 권장 사항:
      1. 기존 캐싱 전략 복원: AuthInterceptor가 빠른 메모리 접근을 할 수 있도록 AtomicReference를 이용한 메모리 캐싱 전략을 복원하는 것이 좋습니다.
      2. AuthInterceptor 개선: AuthInterceptorsuspend 함수가 아니기 때문에 runBlocking을 사용해야 하지만, TokenAuthenticator는 이미 suspend 컨텍스트에서 동작할 수 있습니다. AuthInterceptor에서 runBlocking을 사용해야 한다면, 최소한 TokenDataStore가 토큰을 메모리 캐싱하여 runBlocking 내부에서 디스크 I/O가 발생하지 않도록 해야 합니다.
  • setTokens 네이밍 컨벤션: saveTokens에서 setTokens로 변경된 것은 좋습니다. DataStore의 edit과 맥락을 같이하여 일관성이 있습니다.

  • getTokens()만 남기는 것의 장단점: getAccessToken()이나 getRefreshToken()과 같은 개별 함수를 없애고 getTokens()만 남긴 것은 코드를 간결하게 만들 수 있지만, 특정 토큰만 필요한 경우 매번 getTokens().accessToken과 같이 접근해야 하므로 가독성이나 편리성 면에서 약간의 트레이드오프가 있을 수 있습니다. 하지만 현재 구현에서는 큰 문제는 없어 보입니다.

core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt

  • 트랜잭션 관리: submitOnboarding에서 사용자 정보 업데이트(myPageApi.updateMember)와 AI 캐릭터 생성(aiCharacterApi.createCharacter)을 연달아 호출하는 패턴은 일종의 트랜잭션으로 볼 수 있습니다. 현재는 두 API 호출 중 하나라도 실패하면 Result.failure를 반환합니다. 이는 적절합니다.
  • Domain Model 매핑: OnboardingInput 도메인 모델을 UpdateMemberRequestDtoCreateCharacterRequestDto 네트워크 DTO로 매핑하는 방식은 도메인 계층과 데이터 계층의 관심사를 분리하는 좋은 예시입니다.

core/domain/src/main/java/kr/co/call/domain/model/onboarding/OnboardingInput.kt

  • Data Class var 사용 지양: data class에는 기본적으로 val을 사용하는 것이 불변성을 유지하고 예측 가능한 동작을 하는 데 좋습니다. 현재 OnboardingInput 및 하위 데이터 클래스들은 모두 val로 선언되어 있어 모범 사례를 따르고 있습니다. 훌륭합니다.

Jetpack Compose

app/src/main/java/kr/co/call/callfromai/AppScreen.kt

  • collectSideEffect 사용: viewModel.collectSideEffect를 사용하여 AppSideEffect.NavigateToLogin에 따라 appNavigator.replaceAll(LoginNavKey)를 호출하는 방식은 Orbit MVI 패턴을 올바르게 적용한 것입니다. UI가 직접 ViewModel의 상태를 변경하지 않고, ViewModel이 발생시킨 단발성 이벤트를 처리합니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/screen/MyPageScreen.kt

  • onNavigateToLogin 전달: MyPageScreen에서 onNavigateToLogin 람다를 파라미터로 받아서 MyPageSideEffect.NavigateToLogin 발생 시 호출하는 것은 State Hoisting 원칙을 잘 지킨 것입니다. MyPageScreen은 로그인 화면으로 이동하는 로직을 직접 알 필요 없이, 상위 컴포넌트(AppScreen)로부터 전달받은 콜백을 호출하기만 하면 됩니다.
  • Toast 사용: MyPageSideEffect.ShowMessage에 따라 LocalContext.current를 사용하여 Toast를 띄우는 것은 적절한 Side Effect 처리입니다. UI 상태에 영향을 주지 않는 단발성 메시지 표시에 좋습니다.
  • 다이얼로그 로딩 상태 관리: 로그아웃/계정 삭제 다이얼로그의 onConfirmonDismiss 람다에서 state.authStatus != LoadStatus.Loading을 체크하여 중복 클릭을 방지하고 로딩 중에는 다이얼로그를 닫지 못하게 하는 것은 훌륭한 UX 개선입니다. LoadStatus를 사용하여 로딩/에러 상태를 Boolean 대신 관리하는 좋은 예시입니다.
  • NavigateToLanding 제거: MyPageSideEffect.NavigateToLandingNavigateToLogin으로 대체하고, 에러 메시지 처리를 ShowMessage로 분리한 것은 더 일관성 있고 유연한 Side Effect 관리 방식입니다.

feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding1Screen.kt & Onboarding2Screen.kt

  • Enum 사용으로 타입 안전성 강화: job 필드를 String 대신 MemberJobCharacterJob Enum으로 관리하도록 변경한 것은 매우 좋은 타입 안전성 개선입니다. 하드코딩된 문자열 사용을 줄이고 유효하지 않은 값의 입력을 방지할 수 있습니다.
  • gender, imageUrl 정보 추가: Onboarding1StateOnboarding2StategenderimageUrl 정보를 추가하고, onNextClick 콜백을 통해 ViewModel로 전달하는 것은 UI 상태가 ViewModel에서 관리되는 데이터를 정확하게 반영하도록 하는 좋은 변경입니다.
  • verticalScroll 추가: Onboarding2ScreenverticalScroll을 추가한 것은 화면 내용이 길어질 경우 스크롤이 가능하게 하여 UI 접근성을 높이는 개선입니다.
  • 프로필 이미지 URL 상수화: TemporaryProfileImageData.kt에서 DEFAULT_PROFILE_IMAGE_URL을 상수로 정의한 것은 하드코딩된 문자열을 피하는 좋은 방법입니다.

feature/onboarding/impl/src/main/java/kr/co/call/impl/screen/Onboarding5Screen.kt & Onboarding6Screen.kt

  • isLoading 상태 반영: Onboarding5ScreenOnboarding6Screen에서 isLoading 파라미터를 추가하고, 버튼 활성화 여부나 클릭 이벤트 처리에 반영한 것은 UI가 로딩 상태를 명확히 사용자에게 보여주고 중복 액션을 방지하는 좋은 방법입니다.
  • Onboarding5ScreenonNextClick 변경: viewModel.submitOnboarding(preferTime)을 직접 호출하도록 변경된 것은 온보딩 최종 제출 로직이 ViewModel에서 시작되도록 하여 비즈니스 로직과 UI 로직을 분리합니다.

Repository/DataSource

core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt

  • tokenDataStore.saveTokens -> tokenDataStore.setTokens: TokenDataStore의 메서드 이름 변경에 맞춰 업데이트되었습니다. 좋습니다.

core/data/src/main/java/kr/co/call/data/util/SafeApiResult.kt

  • runRepositoryCatching 유틸 함수: Repository 계층에서 Result를 일관성 있게 반환하도록 runRepositoryCatching 함수를 추가한 것은 코드 중복을 줄이고 에러 핸들링을 표준화하는 훌륭한 패턴입니다. toAppResult()를 통해 도메인 예외로 변환하는 것도 적절합니다.

core/network/src/main/java/kr/co/call/network/api/AICharacterApi.kt & MyPageApi.kt

  • 새로운 API 인터페이스 추가: AICharacterApiMyPageApi를 추가하여 각 도메인에 맞는 API를 분리한 것은 클린 아키텍처 원칙에 부합하며, 의존성 주입 시에도 명확한 역할을 부여합니다.

core/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.kt & TokenAuthenticator.kt

  • TokenDataStore 접근 방식 변경: AuthInterceptorTokenAuthenticator 모두 tokenDataStore.getTokens()를 사용하도록 변경되었습니다. 앞서 TokenDataStore 섹션에서 언급했듯이, AuthInterceptor에서 runBlocking과 함께 DataStore를 직접 읽는 것은 성능 저하를 야기할 수 있습니다. 이 부분에 대한 개선이 필요합니다.
  • authSessionManager.notifySessionExpired() 호출: TokenAuthenticator에서 401 에러 발생 시 tokenDataStore.clearTokens()와 함께 authSessionManager.notifySessionExpired()를 호출하는 것은 전역적인 세션 만료 처리를 위한 핵심 로직입니다. AppViewModel에서 이 이벤트를 구독하여 로그인 화면으로 이동하는 흐름이 자연스럽게 연결됩니다.

ViewModel

app/src/main/java/kr/co/call/callfromai/AppViewModel.kt

  • 전역 세션 만료 처리: AuthSessionManager.sessionExpired를 구독하여 AppSideEffect.NavigateToLogin을 post하는 것은 앱 전반에 걸친 로그인 상태 관리를 위해 매우 견고하고 올바른 방식입니다.

feature/mypage/impl/src/main/java/kr/co/call/impl/viewmodel/MyPageViewModel.kt

  • KakaoLogoutManager 주입: Hilt를 통해 KakaoLogoutManager를 주입받아 사용하는 것은 Hilt DI 생성자 주입 원칙을 준수하고, 소셜 로그인 관련 로직을 분리하는 좋은 방법입니다.
  • 로그아웃/탈퇴 비즈니스 로직:
    • Loading 상태 관리: state.authStatusLoadStatus.Loading으로 업데이트하고, 성공/실패 시 LoadStatus.Idle로 되돌리는 것은 사용자에게 피드백을 주고 중복 실행을 막는 좋은 패턴입니다.
    • 다중 로그아웃 처리: 서버 로그아웃 (myPageRepository.logout()) 뿐만 아니라 카카오 SDK 로그아웃 (kakaoLogoutManager.logout())까지 처리하는 것은 완전한 로그아웃 경험을 제공하기 위해 필수적입니다.
    • Result 핸들링: onSuccessonFailure 블록을 통해 네트워크/카카오 SDK 호출 결과를 처리하고, 에러 메시지를 MyPageSideEffect.ShowMessage로 UI에 전달하는 것은 견고한 에러 핸들링입니다.
    • CancellationException 재전파: CancellationException은 항상 상위 코루틴으로 전파되도록 throw하는 것은 올바른 코루틴 취소 메커니즘을 따르는 것입니다.
  • postSideEffect 남용 여부: 현재 MyPageViewModel에서 postSideEffect는 UI로의 네비게이션, 다이얼로그 표시, 메시지 토스트와 같이 단발성 이벤트에만 사용되고 있습니다. 이는 postSideEffect의 올바른 사용 사례이며, 남용으로 보이지 않습니다.

feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LandingViewModel.kt

  • tokenDataStore.getTokens().accessToken 사용: LandingViewModel에서도 tokenDataStore.getTokens().accessToken을 사용하여 토큰 존재 여부를 확인합니다. 이 역시 TokenDataStore의 성능 이슈와 관련하여 runBlocking 또는 flow.first()에 의한 잠재적 블로킹 문제를 가지고 있습니다. 스플래시 화면에서는 허용될 수 있지만, 앞서 언급한 TokenDataStore의 개선이 이루어지면 더 좋습니다.

FCM/SSE/실시간 통신 (N/A)

이 PR에서는 직접적인 FCM/SSE/실시간 통신 관련 코드가 보이지 않습니다. 하지만 AuthSessionManager를 통해 전역 세션 만료를 처리하는 방식은 이러한 실시간 통신이 인증에 의존하는 경우 유용하게 활용될 수 있습니다.


결론 (Summary)

이 PR은 전반적으로 높은 품질의 코드 개선을 포함하고 있습니다. 특히 MVI 아키텍처의 SideEffect 활용, Repository 계층의 견고한 에러 핸들링, Jetpack Compose UI의 로딩 상태 관리 및 타입 안전성 강화는 매우 긍정적입니다.

가장 중요한 개선 사항은 TokenDataStore의 성능 최적화입니다. AuthInterceptor가 매 네트워크 요청마다 디스크 I/O를 유발하지 않도록, TokenDataStore에 내부 메모리 캐싱 로직을 다시 추가하는 것을 강력히 권장합니다.

그 외 \ No newline at end of file 제거, MyPageRepositoryImpldeleteAccount 내 불필요한 try-catch 제거는 쉽게 적용할 수 있는 부분입니다.

위에 제시된 사항들을 검토하시고 반영해 주시면 더욱 완성도 높은 코드가 될 것입니다. 잘하셨습니다!

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요! 시니어 Android 개발자로서 GitHub PR을 리뷰해 드리겠습니다.

이번 PR은 온보딩 기능 추가, 마이페이지 로그아웃/회원 탈퇴 기능 구현, 그리고 인증 토큰 관리 로직 개선 등 여러 중요한 변경 사항을 포함하고 있네요. 전체적으로 비즈니스 로직과 UI 상태 관리에 ResultLoadStatus를 적극적으로 활용하고, Jetpack Compose의 상태 관리 원칙을 잘 준수하려는 노력이 돋보입니다. 특히, 여러 비동기 작업에 대한 isLoading 상태 관리와 다이얼로그에서 중복 호출 방지 로직은 매우 훌륭합니다.

몇 가지 개선점과 질문 사항들을 정리해 보았습니다.


긍정적인 변경 사항 (Good Points)

  1. Repository 레이어의 견고한 에러 핸들링:
    • runRepositoryCatching, safeApiCallUnit, safeEmptyApiCall과 같은 유틸리티를 도입하여 Retrofit 통신 에러를 Result<T>로 래핑하고 도메인 예외로 변환하는 방식이 체계적입니다. 이는 Repository 레이어의 신뢰도를 높이고 ViewModel에서 에러 처리를 간결하게 할 수 있게 합니다.
    • MyPageRepositoryImpl, OnboardingRepositoryImpl에서 이 패턴을 잘 적용하고 있습니다.
  2. ViewModel의 UI 상태 관리 및 사이드 이펙트 처리:
    • MyPageViewModel에서 authStatus: LoadStatus를 도입하여 로그아웃/탈퇴와 같은 비동기 작업의 로딩 상태를 명확하게 관리하는 점이 좋습니다.
    • 다이얼로그 (MyPageLogoutDialog, MyPageDeleteAccountDialog)에서 state.authStatus != LoadStatus.Loading 조건을 통해 중복 액션을 방지하는 로직은 사용자 경험 측면에서 매우 훌륭합니다.
    • MyPageSideEffect.ShowMessage를 통해 Toast 메시지를 컴포저블에서 한 번만 표시하도록 처리하는 방식은 Orbit의 사이드 이펙트 활용 모범 사례입니다.
    • AppViewModel에서 AuthSessionManager.sessionExpired Flow를 구독하여 전역적으로 로그인 화면으로 이동하는 사이드 이펙트를 발행하는 방식도 앱의 일관된 인증 상태 관리에 기여합니다.
  3. Jetpack Compose 상태 호이스팅 및 불필요한 리컴포지션 방지:
    • Onboarding5Screen, Onboarding6ScreenisLoading 파라미터를 추가하고 이를 통해 버튼 활성화 상태를 제어하는 방식은 상태 호이스팅과 UI 반응성 관리를 잘 보여줍니다.
    • Onboarding1Screen, Onboarding2Screen에서 selectedJob 필드를 String 대신 enum 타입 (MemberJob, CharacterJob)으로 변경하여 타입 안전성을 높인 것은 좋은 개선입니다.
    • Onboarding2ScreenverticalScroll(rememberScrollState())를 추가하여 키보드 등장 시 UI가 잘 스크롤되도록 처리한 점도 사용자 경험 측면에서 좋습니다.
  4. 보안 및 데이터 관리:
    • backup_rules.xmldata_extraction_rules.xml에 DataStore에서 토큰 정보를 백업 제외하도록 설정한 것은 매우 중요한 보안 조치입니다. 앱 재설치 시 자동 로그인 되는 문제나 민감 정보 노출을 방지합니다.

개선이 필요한 부분 및 질문 사항

1. Kotlin 코드 리뷰

  • Coroutine Dispatcher 명시 (AppViewModel)

    • AppViewModelinit 블록에서 authSessionManager.sessionExpired.collect { ... } 코드가 있습니다. Orbit의 intent 블록은 기본적으로 viewModelScope에서 실행되므로, authSessionManager.sessionExpired Flow가 백그라운드 스레드에서 데이터를 발행하는지 확인이 필요합니다. 만약 AuthSessionManager 내부에서 I/O 또는 CPU-bound 작업을 수행한다면, 해당 Flow가 Dispatchers.IO 또는 Dispatchers.Default에서 실행되도록 명시적으로 설정하는 것이 좋습니다.
    • (AuthSessionManager 코드는 PR에 없으므로 가정입니다.)
  • null 안전성 (!! 사용 지양)

    • Onboarding1Screen.kt, Onboarding2Screen.kt에서 jobselectedJob을 처리하는 부분에서 checkNotNull(selectedJob).name과 같이 checkNotNull을 사용하고 있습니다. selectedJobnull이 될 수 있는 시나리오가 없다면 괜찮지만, UI 입력 필드의 유효성 검사 후에만 이 코드가 실행되도록 확실히 보장해야 합니다. 가능하다면 selectedJob?.name ?: "기타"와 같이 ?: 연산자를 활용하여 더 안전하게 처리하는 것을 고려해볼 수 있습니다.
    • TemporaryProfileImageData.ktDEFAULT_PROFILE_IMAGE_URL 상수가 추가되었습니다. 이는 imageUrl에 대한 기본값을 제공하는 좋은 방법이며, 하드코딩된 문자열을 상수로 관리하는 것은 좋습니다.
  • data class에 불필요한 var 사용 여부

    • 제출된 OnboardingInput 관련 data class들은 모두 val로 선언되어 불변성을 유지하고 있습니다. 좋은 코딩 스타일입니다.
    • CreateCharacterRequestDto, UpdateMemberRequestDto 또한 모두 val로 선언되어 있습니다. 좋습니다.
  • Hilt DI 시 생성자 주입 원칙 준수 여부

    • AppViewModel, MyPageViewModel, OnboardingRepositoryImpl, LoginRepositoryImpl 등 모든 Hilt @Inject가 생성자 주입 방식으로 이루어져 있습니다. Hilt 사용 원칙을 잘 준수하고 있습니다.
    • NetworkModule에서 Retrofit API 서비스들을 @Provides @Singleton으로 제공하는 방식도 올바릅니다.
  • 하드코딩된 문자열/매직 넘버

    • LoginRepositoryImpl.kt에서 "로그인 응답 토큰 값이 비어 있습니다"와 같은 문자열은 string.xml 리소스 또는 적절한 도메인 예외 메시지로 관리하는 것을 권장합니다.
    • LandingViewModel.ktSPLASH_DURATION_MILLIScompanion object에 상수로 정의되어 있어 좋습니다.

2. Jetpack Compose 코드 리뷰

  • Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용)

    • MyPageScreen에서 showLogoutDialog, showDeleteAccountDialog, showComingSoon 등의 상태를 remember { mutableStateOf(...) }로 관리하고 있습니다. UI에 따라 가시성이 변경되는 단순한 상태들이므로 적절합니다.
    • onNavigateToLogin과 같은 콜백 함수를 MyPageEntryBuilder에서 MyPageScreen으로 전달하는 것은 상태 호이스팅 원칙에 따라 올바른 방식입니다.
  • UI 상태를 data class로 표현했는지

    • MyPageStatedata class로 잘 정의되어 있으며, LoadStatus를 통해 비동기 작업 상태를 관리합니다. 좋습니다.
  • 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지

    • MyPageStateloadStatusauthStatus 필드를 LoadStatus 타입으로 사용하여 로딩 상태를 명확하게 관리하는 점이 매우 좋습니다.

3. Repository/DataSource 레이어

  • Retrofit 에러 핸들링 (try-catch, Result 래핑)

    • MyPageRepositoryImpl.logout()deleteAccount()Result<Unit>를 반환하고, 내부적으로 runRepositoryCatching 또는 safeApiResultUnit을 사용하여 에러를 Result.failure로 래핑하고 있습니다. 이는 일관된 에러 처리 전략이며 좋습니다.
    • LoginRepositoryImpl.ktsetTokens로 이름 변경 및 saveTokens와의 기능적 통합도 좋습니다.
  • 네트워크 응답과 도메인 모델 매핑 분리 여부

    • OnboardingInput (domain 레이어)과 CreateCharacterRequestDto, UpdateMemberRequestDto (network 레이어)의 분리가 잘 이루어져 있습니다. OnboardingRepositoryImpl에서 이들 간의 매핑을 수행합니다. 좋은 아키텍처입니다.
  • 캐싱 전략 (로컬 DB vs 메모리)

    • TokenDataStore의 변경 사항에 대한 중요한 질문/고려 사항:
      • 기존 TokenDataStoreAtomicReferenceapplicationScope.launch { safeData.collect { ... } }를 통해 메모리 캐싱을 적극적으로 활용했습니다. AuthInterceptor는 이 메모리 캐시에서 accessToken을 가져왔기 때문에 I/O를 유발하지 않았습니다.
      • 변경 후에는 메모리 캐싱 로직이 제거되고, AuthInterceptorTokenAuthenticator 모두 runBlocking { tokenDataStore.getTokens().accessToken } (또는 getTokens())를 호출합니다. dataStore.data.first()는 DataStore 파일에서 값을 읽는 I/O 작업입니다.
      • 질문: AuthInterceptor는 앱의 거의 모든 네트워크 요청에 대해 실행됩니다. runBlocking 내부에서 DataStore I/O를 수행하는 것이 성능에 미치는 영향에 대해 고려해 보셨나요? 기존의 메모리 캐싱 전략을 제거한 의도가 궁금합니다. 만약 TokenDataStore가 더 이상 실시간으로 변경될 필요가 없거나, DataStore.data.first()의 성능 오버헤드가 무시할 만하다고 판단하셨다면 괜찮습니다. 하지만 일반적인 HTTP 요청의 Interceptor에서 I/O를 매번 수행하는 것은 잠재적인 병목이 될 수 있습니다.
      • 대안 고려: AuthSessionManager를 통해 토큰 정보를 StateFlow로 노출하고, AuthInterceptor는 이 StateFlowvalue를 가져오는 방식으로 변경하여 runBlocking 내부의 I/O를 제거하는 것을 고려해볼 수 있습니다. 하지만 이 경우 AuthInterceptorStateFlow를 구독하는 등 비동기적으로 동작하기 어렵다는 OkHttp Interceptor의 한계가 있습니다. 현 구조에서 runBlocking + first()가 일반적인 해결책이긴 합니다. 기존처럼 AtomicReference를 통한 인메모리 캐싱을 유지하고 AuthInterceptor에서만 사용하도록 복구하는 것이 더 효율적일 수 있습니다.
    • MyPageRepositoryImpl.logout(): 서버에 로그아웃 요청을 보내는 것 외에 tokenDataStore.clearTokens()를 호출하여 로컬 토큰을 삭제하는 것은 올바른 로직입니다.
    • MyPageRepositoryImpl.deleteAccount(): 회원 탈퇴 후에도 tokenDataStore.clearTokens()를 호출하는 것은 좋습니다. onSuccess 블록에서 try-catch로 감싼 것은 CancellationException을 다시 던지기 위한 좋은 패턴입니다.

4. ViewModel

  • UI 상태와 비즈니스 로직 분리

    • MyPageViewModel에서 로그아웃/탈퇴 비즈니스 로직을 처리하고, 결과에 따라 MyPageSideEffect.NavigateToLogin 또는 MyPageSideEffect.ShowMessage와 같은 UI 이벤트를 발행하는 방식은 잘 분리되어 있습니다.
    • AppViewModel도 앱 전역 상태 (sessionExpired)를 비즈니스 로직으로 처리하고, AppSideEffect.NavigateToLogin으로 UI 이펙트를 발행합니다.
  • Orbit ContainerHost 패턴 준수 여부

    • 모든 ViewModel이 ContainerHost를 구현하고 container를 통해 initialState를 설정하는 등 Orbit 패턴을 잘 준수하고 있습니다.
  • intent 내부에서 상태 변경(reduce)만 수행하는지

    • MyPageViewModellogout()deleteAccount() 메서드에서 reduce { state.copy(authStatus = LoadStatus.Loading) }와 같이 reduce 블록 내에서만 상태 변경을 수행하고 있습니다. 좋습니다.
  • postSideEffect 남용 여부

    • postSideEffect는 로그인 화면 이동, 토스트 메시지 표시 등 일회성 UI 이벤트에만 사용되고 있습니다. 남용되지 않고 적절하게 사용되고 있습니다.
  • blocking 작업을 intent 내부에서 직접 수행하지 않는지

    • MyPageViewModellogout()deleteAccount() 내부에서 Repository 호출은 suspend 함수이므로 intent 블록 내에서 viewModelScope의 Coroutine에서 비동기적으로 실행됩니다. blocking 작업을 직접 수행하지 않습니다.
    • AppViewModelinit 블록 내 collect도 suspending 함수이므로 메인 스레드를 블로킹하지 않습니다.

5. FCM/SSE/실시간 통신 관련 코드

  • 해당 PR에는 FCM/SSE/실시간 통신 관련 코드가 포함되어 있지 않습니다.

결론 및 최종 권고

전반적으로 이번 PR은 새로운 기능을 구현하면서 아키텍처 원칙 (클린 아키텍처, MVI with Orbit, Jetpack Compose)을 매우 잘 준수하고 있습니다. 특히 에러 핸들링, 로딩 상태 관리, UI와 비즈니스 로직 분리, Compose의 상태 호이스팅 등 핵심적인 부분에서 모범적인 코드를 보여주고 있습니다.

몇 가지 질문과 고려 사항 (특히 TokenDataStore의 캐싱 전략 변경)에 대한 논의가 필요하지만, 이는 기능 구현 자체의 문제라기보다는 아키텍처 및 성능에 대한 최적화 또는 의도 확인에 가깝습니다.

다음 사항에 대한 추가 검토 또는 논의를 권장합니다:

  1. TokenDataStore의 메모리 캐싱 제거에 따른 AuthInterceptor의 성능 영향: runBlocking 내부에서 매번 DataStore I/O를 수행하는 것에 대한 의도와 성능 저하 가능성에 대한 검토. (제일 중요한 부분이라고 생각합니다.)
  2. MyPageApi.logout()deleteAccount()의 Retrofit 응답 타입: ApiResponse<Any> 대신 Response<Unit> 또는 ApiResponse<Unit>이 더 정확할 수 있습니다. (이는 @POST("auth/logout") suspend fun logout()처럼 ApiResponse<Any>가 아닌 Unit을 반환하는 함수에서는 safeEmptyApiCall을 사용할 수 있기에 변경하는 것이 좋습니다.)
  3. 하드코딩된 문자열 처리: LoginRepositoryImpl 내의 에러 메시지 등 사용자에게 표시될 수 있는 문자열은 string.xml로 이동하는 것을 권장합니다.
  4. AppViewModel의 Flow Dispatcher 명시: authSessionManager.sessionExpired Flow의 실행 Dispatcher 확인 및 필요시 명시적 지정.

이 외의 변경 사항들은 매우 훌륭하며, 위 항목들에 대한 논의 및 개선이 이루어진다면 PR 승인에 문제가 없을 것 같습니다.

수고 많으셨습니다!

@codebidoof codebidoof left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아원이가 만든 메서드 사용하는 식으로 고쳐주면 될 것 같아!

Comment on lines +8 to +19
//예외처리파일
/**
* Repository 작업에서 발생한 예외를 앱의 도메인 예외로 변환한다.
* 코루틴 취소와 치명적인 [Error]는 [toAppResult]에서 다시 전파된다.
*/
suspend fun <T> runRepositoryCatching(
block: suspend () -> T,
): Result<T> =
runCatching {
block()
}.toAppResult()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 추가할 필요 없이 그냥 아원이가 만들어놓은거 쓰면 될 것 같아!

Comment on lines 27 to 54

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아원이가 safeApiCall류 메서드들 반환타입 전부 Result로 맞춰 놨으니까 여기서 runCatching이랑 toAppResult() 쓰지 말고 safeApiCall만 호출해서 성공하면 dto에 오는 토큰 datastore에 캐싱하는 식으로 해 주면 될 것 같아

Comment on lines +64 to +80
suspend fun safeEmptyApiCall(
parser: ErrorResponseParser,
call: suspend()-> Response<Unit>,
){
try {
val response=call()
if (!response.isSuccessful){
throw HttpException(response)
}
}catch (e: CancellationException){
throw e
}catch (e: ApiException){
throw e
}catch(e: HttpException){
throw e.toApiException(parser)
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 추가할 필요 없이 그냥 아원이 꺼 쓰면 될 것 같아~!

@codebidoof codebidoof assigned codebidoof and dada4679 and unassigned codebidoof Jul 31, 2026
@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요, PR 리뷰 요청해주셔서 감사합니다. PR 내용을 보니, 앱 전반에 걸쳐 인증 세션 만료 처리, 온보딩 기능 추가, 마이페이지 로그아웃/탈퇴 기능 개선 등 중요한 업데이트가 많네요. 전반적으로 코드 품질 향상을 위한 노력과 Jetpack Compose, Orbit MVI 패턴 적용이 잘 되어 있어 긍정적입니다. 몇 가지 개선 사항과 고려할 점들을 아래와 같이 정리했습니다.


1. 전반적인 피드백 (General Feedback)

  • 긍정적인 부분:

    • AuthSessionManager를 도입하여 세션 만료 이벤트를 앱 전역으로 전파하고 AppViewModel에서 이를 구독하여 로그인 화면으로 이동시키는 패턴은 매우 좋습니다.
    • TokenDataStore에서 토큰 백업 제외 처리는 보안상 중요하며 잘 반영되었습니다.
    • 새로운 Repository 구현체들(LoginRepositoryImpl, MyPageRepositoryImpl, OnboardingRepositoryImpl)에서 safeApiResult 헬퍼 함수를 사용하여 Retrofit 에러를 Result로 래핑하고 도메인 모델과 매핑하는 방식이 일관성 있게 잘 적용되어 있습니다.
    • 마이페이지에서 로그아웃/회원 탈퇴 시 LoadStatus를 사용하여 UI 상태를 관리하고, 다이얼로그에서 중복 액션을 방지하는 로직이 적용된 점은 사용자 경험 측면에서 좋습니다.
    • 카카오 SDK의 콜백 기반 API를 코루틴 suspendCancellableCoroutine으로 래핑한 KakaoLogoutManager 구현은 매우 깔끔하고 모범적인 방식입니다.
    • 온보딩 관련 새로운 도메인 모델, Repository, DTO 등이 추가되면서 관심사 분리가 잘 되어 있습니다.
  • 개선이 필요한 부분:

    • TokenDataStore의 캐싱 전략 변경으로 인해 발생할 수 있는 네트워크 인터셉터의 성능 저하 문제가 보입니다. (아래 상세 설명)
    • 온보딩 화면의 하드코딩된 문자열 (직업, MBTI 옵션 등)은 추후 관리 및 지역화에 문제가 될 수 있습니다.

2. 상세 코드 리뷰

2.1 Kotlin 코드 리뷰

  • TokenDataStore.kt - 캐싱 전략 변경 관련 (Critical)

    • 이슈: 이전 TokenDataStoreAtomicReference를 사용해 메모리에 토큰을 캐시하고, applicationScope에서 safeData.collect으로 DataStore 변경사항을 구독하여 캐시를 업데이트하는 방식이었습니다. 이로 인해 AuthInterceptorgetCachedAccessToken()을 통해 Disk I/O 없이 빠르게 토큰을 가져올 수 있었습니다. 하지만 이번 PR에서는 이 캐싱 로직이 완전히 제거되어, AuthInterceptorTokenAuthenticator에서 tokenDataStore.getTokens().accessToken을 호출할 때마다 runBlocking 내부에서 dataStore.data.first()를 통해 Disk I/O가 발생하게 됩니다.
    • 문제점: AuthInterceptor는 모든 인증된 네트워크 요청마다 호출됩니다. 매번 runBlocking으로 Disk I/O를 수행하면 네트워크 요청의 Latency가 증가하여 앱의 전반적인 성능에 부정적인 영향을 줄 수 있습니다.
    • 제안:
      1. Disk I/O 최소화: AuthInterceptor가 토큰을 가져올 때 Disk I/O를 직접 수행하지 않도록 TokenDataStoreFlow<StoredTokens>를 노출하고, AuthInterceptor가 이를 구독하여 메모리 캐시를 유지하는 방식으로 다시 변경하는 것을 고려해야 합니다. (혹은 AuthInterceptorCoroutineScope를 주입하여 async로 토큰을 가져오게 하는 방법도 있지만, runBlocking 제거가 더 큰 목표입니다.)
      2. AuthInterceptorsuspend 함수가 아니기 때문에 runBlocking이 불가피할 수 있습니다. 하지만 dataStore.data.first()suspend 함수이므로 runBlocking이 필요하게 됩니다. 만약 TokenDataStore 내부에서 MutableStateFlow 등을 사용하여 토큰을 미리 로드하고 AuthInterceptor에서는 stateFlow.value로 접근하게 한다면 runBlocking을 피하고 Disk I/O를 줄일 수 있을 것입니다.
  • backup_rules.xml, data_extraction_rules.xml - DataStore 제외 (Good)

    • preferences_pb 파일을 백업에서 제외한 것은 리프레시 토큰 등 민감한 인증 정보가 유출되거나 앱 재설치 시 자동 로그인되는 문제를 방지하는 좋은 보안 조치입니다. 잘하셨습니다.
  • AppViewModel.kt - init 블록 내 Flow collect (Good)

    • init 블록에서 authSessionManager.sessionExpired.collect을 호출하는 것은 ViewModel의 라이프사이클에 맞춰 Flow를 구독하는 올바른 방법입니다. intent { ... } 블록 내에서 실행되므로 viewModelScope에 의해 관리됩니다.
  • Onboarding1Screen.kt, Onboarding2Screen.kt - checkNotNull 사용 (Could Fix)

    • checkNotNull(selectedJob).name과 같이 !! 대신 checkNotNull을 사용한 점은 좋습니다. canMoveNext 조건에 의해 selectedJobnull이 아닐 것이라고 가정하는데, 이 가정이 항상 유효한지 다시 한번 확인하고, 만약 예외 상황이 발생할 가능성이 있다면 ? 안전 호출과 적절한 폴백 처리를 고려해볼 수 있습니다. 현재 로직에서는 canMoveNext로 인해 selectedJobnull일 수 없어 보여 큰 문제는 아닙니다.
  • 하드코딩된 문자열 (Should Fix)

    • Onboarding1Screen.kt, Onboarding2Screen.kt에서 직업("대학생", "직장인", "기타") 및 MBTI(Mbti.entries.map { it.name }) 등 UI에 표시되는 옵션들이 문자열 리터럴로 되어 있습니다.
    • 제안: 이들은 strings.xml에 정의된 문자열 리소스(R.string.job_student 등)로 대체하여 지역화 및 유지보수성을 높이는 것이 좋습니다. MemberJobCharacterJob enum class에 @StringRes 애노테이션을 사용하여 리소스 ID를 연결하는 방식도 고려해볼 수 있습니다.

2.2 Jetpack Compose 코드 리뷰

  • MyPageScreen.kt - 다이얼로그에서 isLoading 상태 반영 (Good)

    • 로그아웃/계정 삭제 다이얼로그의 onConfirmonDismiss 람다에 state.authStatus != LoadStatus.Loading 조건을 추가하여 로딩 중에는 다이얼로그를 닫거나 중복 요청을 보내지 않도록 한 점은 사용자 경험과 UI 무결성을 위해 매우 좋은 개선입니다.
  • Onboarding5Screen.kt - isLoading Prop 사용 (Good)

    • Onboarding5Screen에서 isLoading Prop을 받아 "다음" 버튼의 활성화 여부를 제어한 것은 UI 상태 관리가 잘 되어 있음을 보여줍니다.
  • Onboarding6Screen.kt - isLoading Prop 하드코딩 (Should Fix)

    • Onboarding6Screen에서 isLoading = false로 하드코딩되어 있습니다. 만약 "지금 전화할래", "나중에 전화할래" 버튼이 특정 API 호출을 유발하고 그 호출이 비동기적으로 처리된다면, 해당 버튼들도 isLoading 상태를 반영해야 합니다.
    • 제안: Onboarding6Screen에서도 실제 비즈니스 로직에 따라 isLoading 상태를 ViewModel로부터 받아와 버튼 활성화 여부를 제어하도록 수정해야 합니다.
  • OnboardingScreen들의 상태 Hoisting (Generally Good, but minor points)

    • Onboarding1ScreenOnboarding2Screen은 내부적으로 rememberSaveable을 사용하여 UI 상태를 관리하고, "다음" 버튼 클릭 시 onNextClick 람다를 통해 모든 현재 상태 값을 ViewModel로 전달하는 방식을 사용하고 있습니다. 이는 다단계 온보딩 흐름에서 각 스크린이 자체적인 입력 상태를 효율적으로 관리하면서도 최종적으로 ViewModel에 통합될 수 있도록 하는 합리적인 접근입니다.
    • 사소한 제안: onNextClick 람다의 파라미터가 많아지는 경향이 있습니다. 만약 온보딩 스크린이 더 많아지고 파라미터가 복잡해진다면, OnboardingInput과 같은 데이터 클래스를 onNextClick의 단일 파라미터로 사용하는 것도 고려해볼 수 있습니다. 현재는 큰 문제는 아닙니다.
  • Onboarding1Screen.kt, Onboarding2Screen.kt - ProfileChoiceStategender (Could Fix)

    • ProfileChoice 컴포넌트에서 selectedGenderGender.MALE 또는 Gender.FEMALE로 받고 있지만, OnboardingViewModelupdateGender는 String을 받습니다. enum의 name을 사용하는 것으로 보입니다. 이는 일관성을 위해 enum 자체를 넘겨주는 것이 더 명확할 수 있습니다.
    • 제안: ViewModel에서 updateGender(gender: Gender)와 같이 enum을 직접 받아서 처리하거나, ProfileChoiceState를 업데이트할 때 selectedGender.name으로 변환하는 로직을 명확히 하는 것이 좋습니다.

2.3 Repository/DataSource Layer

  • LoginRepositoryImpl.kt, MyPageRepositoryImpl.kt, OnboardingRepositoryImpl.kt - safeApiResult 사용 (Excellent)

    • Retrofit API 호출을 safeApiResultsafeApiResultUnit으로 래핑하여 Result 타입을 반환하도록 변경한 것은 매우 훌륭합니다. 이로써 네트워크 통신 에러 처리, CancellationException 방지, 비즈니스 로직 분리가 명확해졌습니다. mapCatching을 사용하여 도메인 모델 매핑 시 발생할 수 있는 예외까지 Result로 처리하는 점도 좋습니다.
  • MyPageRepositoryImpl.kt - getMyProfile (Should Fix)

    • getMyProfile이 여전히 임시 구현(delay(500))으로 하드코딩된 데이터를 반환하고 있습니다.
    • 제안: 실제 API 연동이 완료되는 시점에 맞추어 이 부분도 safeApiResult를 적용하여 실제 서버 데이터를 가져오도록 수정해야 합니다.
  • MyPageRepositoryImpl.kt - deleteAccount의 토큰 삭제 에러 핸들링 (Good)

    • onSuccess 블록에서 tokenDataStore.clearTokens()를 호출하고 onFailure 블록에서 Timber로 경고 로깅을 한 것은 좋습니다. CancellationException을 다시 던지는 처리도 올바릅니다.
  • TokenDataStore.kt - DataStore Flow catch 로직 제거 (Should Fix)

    • 이전에는 safeData Flow에서 IOException 발생 시 emit(emptyPreferences())로 처리하여 앱 크래시를 방지했습니다. 이 로직이 제거되면서 DataStore 파일 읽기/쓰기 시 IOException 등의 에러가 발생하면 앱이 비정상 종료될 가능성이 있습니다.
    • 제안: TokenDataStoregetTokens(), setTokens(), clearTokens() 등에서 DataStore 관련 IOExceptiontry-catch로 처리하여 Result 타입으로 반환하거나, 특정 예외 발생 시 Timber 로깅 후 기본값(empty tokens)을 반환하여 앱이 죽지 않도록 방어 로직을 추가하는 것을 고려해야 합니다.

2.4 ViewModel

  • AppViewModel.kt - 세션 만료 처리 (Excellent)

    • AuthSessionManager를 주입받아 sessionExpired Flow를 구독하고 AppSideEffect.NavigateToLoginpostSideEffect하는 방식은 앱 전역에서 인증 상태를 중앙 집중적으로 관리하는 모범적인 패턴입니다.
  • MyPageViewModel.kt - 로그아웃/탈퇴 로직 (Good)

    • logout()deleteAccount() 함수 내에서 Result 반환값을 onSuccess / onFailure로 처리하고, 사용자에게 피드백(ShowMessage)을 주는 로직이 잘 구현되어 있습니다. LoadStatus를 사용하여 로딩 상태를 관리하는 점도 좋습니다.
    • KakaoLogoutManager를 통해 카카오 SDK 로그아웃까지 연동한 점도 꼼꼼한 처리입니다.
  • LoginViewModel.kt - 로그인 분기 처리 (Good)

    • 로그인 후 needsTermsAgreementneedsOnboarding 값에 따라 적절한 AppSideEffectpostSideEffect하는 로직이 깔끔하게 구현되어 있습니다. Timber 로깅도 추가되어 디버깅에 도움이 될 것입니다.
  • OnboardingViewModel.kt (부분적으로 보이는 내용 기반) - submitOnboarding (Good)

    • Onboarding5Screen에서 onboardingViewModel.submitOnboarding(preferTime)을 호출하여 API 호출을 수행하는 것으로 보입니다. uiState.submitStatus == LoadStatus.Loading을 통해 로딩 상태를 관리하는 점이 좋습니다.

2.5 FCM/SSE/실시간 통신 관련 코드

  • 해당 PR에는 FCM/SSE/실시간 통신 관련 코드가 직접적으로 포함되어 있지 않아 리뷰 대상에서 제외합니다.

3. 총평

이번 PR은 AuthSessionManager를 통한 전역 인증 상태 관리, safeApiResult를 활용한 Repository 계층의 에러 핸들링 일관성 확보, Jetpack Compose의 UI 상태 관리 모범 사례 적용 등 여러 면에서 높은 품질의 개선을 이루었습니다.

특히 TokenDataStore의 캐싱 전략 변경으로 인한 성능 저하 우려(AuthInterceptor의 Disk I/O)는 Critical 한 문제로, 우선적으로 해결되어야 할 것으로 보입니다. 이 외의 제안 사항들은 대부분 Should Fix 또는 Could Fix에 해당하는 개선점이므로, 개발 일정과 우선순위를 고려하여 반영해 주시면 좋겠습니다.

다시 한번 수고하셨습니다! 👍

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@app/src/main/res/xml/data_extraction_rules.xml`:
- Around line 12-13: Update the data extraction rules to enable the
device-transfer configuration and exclude
datastore/call_from_ai_token_prefs.preferences_pb there as well as in the
existing cloud-backup rules, preserving the token exclusion for both backup
paths.
🪄 Autofix (Beta)

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 88d3afa8-a853-4772-873e-3730ddd38700

📥 Commits

Reviewing files that changed from the base of the PR and between bd0d5ab and 3e7c8d3.

📒 Files selected for processing (12)
  • app/src/main/res/xml/backup_rules.xml
  • app/src/main/res/xml/data_extraction_rules.xml
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/OnboardingRepositoryImpl.kt
  • core/domain/src/main/java/kr/co/call/domain/model/onboarding/OnboardingInput.kt
  • core/domain/src/main/java/kr/co/call/domain/repository/OnboardingRepository.kt
  • core/network/src/main/java/kr/co/call/network/api/MyPageApi.kt
  • core/network/src/main/java/kr/co/call/network/util/SafeApiCall.kt
  • feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/model/Job.kt
💤 Files with no reviewable changes (2)
  • core/network/src/main/java/kr/co/call/network/util/SafeApiCall.kt
  • core/domain/src/main/java/kr/co/call/domain/model/onboarding/OnboardingInput.kt
🚧 Files skipped from review as they are similar to previous changes (4)
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/model/Job.kt
  • core/network/src/main/java/kr/co/call/network/api/MyPageApi.kt
  • core/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.kt
  • feature/onboarding/impl/src/main/java/kr/co/call/impl/viewmodel/OnboardingViewModel.kt

Comment on lines +12 to +13
<!-- 리프레시 토큰 등 인증 정보가 클라우드 백업에 포함되면 앱 삭제 후 재설치해도 자동 로그인되는 버그 발생 -->
<exclude domain="file" path="datastore/call_from_ai_token_prefs.preferences_pb"/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== dataExtractionRules 참조 확인 =="
rg -n -C 3 'dataExtractionRules|fullBackupContent' -g 'AndroidManifest.xml' .

echo "== 현재 데이터 추출 규칙 확인 =="
cat -n app/src/main/res/xml/data_extraction_rules.xml

Repository: call-from-ai/CallFromAi_Android

Length of output: 1729


🌐 Web query:

Android 12 device transfer data restoration excluded backup data documentation exclude device transfer

💡 Result:

For devices running Android 12 (API level 31) and higher, the android:allowBackup and android:fullBackupContent attributes are deprecated for controlling device-to-device (D2D) transfers and now apply only to cloud-based backups [1]. To control data exclusion for both cloud backups and D2D transfers on Android 12+, you must use the android:dataExtractionRules attribute in your AndroidManifest.xml [2][3][4]. Configuration Procedure 1. Update AndroidManifest.xml: Add the android:dataExtractionRules attribute pointing to an XML resource file (e.g., @xml/data_extraction_rules) [3][5]. <application... android:dataExtractionRules="@xml/data_extraction_rules"> 2. Create the Rules File: Define your inclusion and exclusion rules within the res/xml/data_extraction_rules.xml file [3][5]. This file allows you to define rules separately for cloud-backup and device-transfer [3][4]. Example Configuration: Key Considerations * D2D Transfers: If the android:dataExtractionRules attribute is not specified, the system defaults to transferring all application data during a D2D migration [2]. * Backward Compatibility: If your app supports Android 11 (API level 30) or lower, you must still maintain android:fullBackupContent and its associated backup rules file to ensure consistent behavior across all supported Android versions [2][1][5]. * Granular Control: You can use the include and exclude tags within both cloud-backup and device-transfer blocks to specify which domains (e.g., file, database, sharedpref, external, root) and specific file paths to include or exclude [3][4]. Note that if you use an include tag, the system will only back up or transfer the specified files, ignoring others by default [6]. For further guidance, consult the official Android documentation on Backup Best Practices and Auto Backup.

Citations:


🌐 Web query:

Android default device transfer includes all app data unless data extraction rules exclude

💡 Result:

The statement is generally correct regarding how Android handles app data during migration. By default, the Android system includes application data in both cloud backups and device-to-device (D2D) transfers unless the application developer explicitly excludes it using configuration rules [1][2]. Key details regarding this process include: 1. Automatic Inclusion: For most apps targeting Android 6.0 (API level 23) or higher, Auto Backup is enabled by default [2]. If no specific backup or data extraction rules are defined, the system will attempt to transfer the app's data during a device migration [1]. 2. Configuration Rules: Developers can control this behavior using XML configuration files. - On Android 12 (API level 31) and higher, developers use the android:dataExtractionRules attribute in the AndroidManifest.xml to point to a configuration file that defines and rules [3][4]. - If the tag is not configured or does not include an explicit exclusion, the default behavior remains to transfer the application's data [1]. - Developers can use these rules to include or exclude specific domains (such as file, database, sharedpref, external, or root) and paths [3][5]. 3. Limitations: Not all app data is guaranteed to transfer [6][7]. The success of a transfer depends on: - The application developer's implementation: If an app does not use the standard Android backup framework or has explicitly excluded its data, that data will not be transferred [6][2]. - System limitations: Certain types of data, such as app data for apps not from the Google Play Store or data from apps that do not utilize Android backup, may not be included [6]. - Version compatibility: Rules for data extraction were refined in Android 12, requiring developers to ensure their backup configurations are compatible across different Android versions [1][4]. In summary, while the system defaults to including app data, developers retain the authority to exclude specific data or prevent it from being backed up/transferred entirely through defined extraction rules [1][4]. Users should be aware that the actual data transferred can vary by app [6][7].

Citations:


🌐 Web query:

Android BackupDataTransport DeviceShadowFileAgent device transfer fullBackupContent all app data without data extraction rules

💡 Result:

The terms you mentioned relate to different layers and concepts within the Android backup and data transfer infrastructure: BackupDataTransport is not a standard public API class but rather refers to the underlying transport mechanism (often implementing the IBackupTransport interface) that handles the movement of backup data to storage or between devices [1][2][3]. DeviceShadowFileAgent is not a documented Android system class or standard API component. It is possible this is an internal identifier or a misunderstanding of how the system "shadows" or handles file-based backups during data migration. FullBackupContent refers to the android:fullBackupContent attribute in an app's AndroidManifest.xml [4][5]. This attribute allows developers to point to an XML resource file that defines rules for including or excluding specific files and directories during Auto Backup and device-to-device transfers [6][4][7]. When an app does not define data extraction rules (or an explicit fullBackupContent/dataExtractionRules configuration), the Android system's default behavior is to include almost all data within the application's local directory (such as /data/data/<package_name>/) in the backup set, provided that android:allowBackup is set to true (or is left as the default true) [4][5]. In the context of device-to-device transfers, the system uses these same rules. If no specific exclusions are defined, the backup agent—which is responsible for the interaction between the application and the backup system—will process all accessible data files unless otherwise restricted by the system's own security policies [8][6]. Because backup and restoration involve an app's private files, failure to explicitly define these rules can lead to the accidental inclusion of sensitive information (like tokens, cache, or device-specific identifiers) in transfers to other devices [4][7].

Citations:


🌐 Web query:

Android frameworks base BackupManagerService shouldIncludeDeviceTransfer device shadow file device transfer not including all app data

💡 Result:

In the Android backup framework, there is no public or standard internal method named "shouldIncludeDeviceTransfer" within the BackupManagerService or related classes. The mechanism for determining what data is included in a device-to-device (D2D) transfer or cloud backup is primarily controlled through the application's backup configuration, specifically the backup_rules.xml file [1][2]. If an application is not including all expected app data during a device transfer, it is typically due to one of the following factors rather than a framework-level "shadow file" issue: 1. Configuration Rules (backup_rules.xml): Developers define which data to include or exclude for cloud backups and device-to-device transfers using the and tags in their configuration XML [1]. If data is missing, it is often because it falls under an excluded domain or path, or because the specific rules were not correctly configured to include the desired data [1][2]. 2. Transport Capabilities: Device-to-device transfer uses a specific transport mechanism (often the D2dTransport) [3][4]. Not all app data is guaranteed to be transferred if it does not meet the requirements of this transport or if the app has opted out of certain types of backups [1][2]. 3. Security and Encryption: Android may exclude data from backups—especially during automated processes—if the data is marked as sensitive or if the device does not meet specific security requirements (such as having a screen lock set for client-side encryption) [1][2]. 4. Backup Agent Implementation: Applications can override default behavior by implementing a BackupAgent [5]. If the agent is not correctly configured to handle full-backup or key-value data, or if it explicitly excludes files during the backup process, the data will not be available for transfer [6][5]. To debug issues where app data is missing after a transfer, developers should verify their backup_rules.xml to ensure proper inclusion of required domains (such as device_file, device_database, or root) [1] and confirm that the BackupAgent is not filtering out the files during the traversal of the backup file tree [6]. You can test this behavior by using the bmgr tool to simulate backups and inspect what data is being handled by the active transport [3][4].

Citations:


device-transfer에서도 토큰 DataStore를 제외하세요.

android:dataExtractionRules="@xml/data_extraction_rules"은 현재 cloud-backup만 적용합니다. device-transfer 규칙은 비활성 상태라 Android 12 이상 기기 간 전송 경로에서 토큰 파일이 포함될 수 있습니다. device-transfer를 활성화하고 datastore/call_from_ai_token_prefs.preferences_pb도 제외하세요.

🤖 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 `@app/src/main/res/xml/data_extraction_rules.xml` around lines 12 - 13, Update
the data extraction rules to enable the device-transfer configuration and
exclude datastore/call_from_ai_token_prefs.preferences_pb there as well as in
the existing cloud-backup rules, preserving the token exclusion for both backup
paths.

@github-actions

Copy link
Copy Markdown

Gemini AI 코드리뷰

안녕하세요, 시니어 Android 개발자입니다. PR 리뷰 요청해주셔서 감사합니다. diff 내용을 꼼꼼히 살펴보았으니, 아래 피드백을 참고하여 더욱 견고하고 유지보수하기 쉬운 코드를 만들어나가시길 바랍니다.


전반적인 평가

이번 PR은 온보딩 플로우 구현과 기존 로그인/로그아웃/회원탈퇴 로직 개선, 그리고 세션 만료 시 전역적인 로그아웃 처리를 위한 중요한 변경사항들을 담고 있습니다. Orbit MVI 패턴과 LoadStatus를 사용하여 비동기 작업 상태를 관리하고, Result 타입을 통해 에러 핸들링을 명확히 한 점이 매우 인상적입니다. 특히 TokenDataStore의 불필요한 메모리 캐싱 로직을 제거하고, TokenAuthenticator에서 세션 만료 시 AuthSessionManager를 통해 전역적으로 알림을 보내는 방식은 앱의 인증 시스템을 크게 개선했다고 생각합니다.

몇 가지 개선할 점과 TODO 주석 처리되지 않은 임시 구현체가 남아있는 부분이 있어 이에 대한 피드백을 드립니다.


코드 리뷰 상세

1. Kotlin 코드

  • Coroutine 사용 시 Dispatcher 명시 여부, viewModelScope/lifecycleScope 오남용
    • AppViewModelinit 블록에서 authSessionManager.sessionExpired.collect를 호출하는 것은 viewModelScope 내에서 안전하게 실행됩니다. Flowcollect는 blocking 작업이 아니며, viewModelScope가 ViewModel 생명주기를 따르므로 적절합니다.
    • AuthInterceptorTokenAuthenticator에서 tokenDataStore 접근 시 runBlocking을 사용한 것은 OkHttp Interceptor/Authenticator의 동기적 특성을 고려할 때 허용 가능한 패턴입니다. DataStorefirst() 호출이 실제 디스크 I/O를 포함하더라도, 이 부분은 인증 흐름의 핵심이므로 트레이드오프를 감수할 수 있습니다.
    • ViewModel 내의 intent 블록에서 호출되는 Repository 함수들은 suspend 키워드를 사용하고 있어 코루틴 컨텍스트가 잘 전파될 것으로 보입니다. Repository 내부에서 Dispatchers.IO를 명시적으로 사용하는 것이 네트워크 통신 시 관례적이지만, Retrofit과 safeApiResult 내부에서 기본적으로 백그라운드 스레드를 사용하므로 이 부분은 현재 코드로도 큰 문제는 없을 것입니다. (다만 safeApiResult 내부에서 withContext(Dispatchers.IO)와 같은 명시적 컨텍스트 전환이 있다면 더욱 좋습니다.)
  • Flow/StateFlow 사용 시 collect 시점 (repeatOnLifecycle 사용 여부)
    • Compose UI에서 viewModel.collectSideEffectstateFlow.collectAsStateWithLifecycle()을 사용하고 있어 Composable의 생명주기에 맞게 안전하게 Flow를 소비하고 있습니다. 아주 좋은 구현입니다.
  • null 안전성 (!! 사용 지양)
    • Onboarding1Screen, Onboarding2Screen에서 checkNotNull(selectedJob)과 같은 !! 사용이 보이지만, canMoveNext 조건에서 selectedJob != null을 먼저 확인하므로 안전하게 사용되고 있습니다. 이처럼 확실한 보장이 있는 곳에서의 !! 사용은 허용 가능합니다.
  • data class에 불필요한 var 사용 여부
    • 새로 추가된 OnboardingInput 도메인 모델과 CreateCharacterRequestDto, UpdateMemberRequestDto 등 모든 data class 필드가 val로 선언되어 있습니다. 불변성을 유지하는 좋은 코딩 습관입니다.
  • Hilt DI 시 생성자 주입 원칙 준수 여부
    • AppViewModel, LoginRepositoryImpl, MyPageRepositoryImpl, OnboardingRepositoryImpl, TokenDataStore, KakaoLogoutManager, MyPageViewModel 등 Hilt를 사용하는 모든 클래스에서 생성자 주입 원칙을 잘 준수하고 있습니다.
  • 하드코딩된 문자열/매직 넘버
    • AppScreen.kt에서 showComingSoon = true가 있는 부분의 "Coming Soon" 텍스트는 현재 보이지 않지만, 만약 이 메시지를 UI에 직접 노출한다면 stringResource로 관리하는 것이 좋습니다.
    • TemporaryProfileImageData.ktDEFAULT_PROFILE_IMAGE_URL 상수를 추가하여 매직 넘버/문자열을 제거한 점은 훌륭합니다.
    • SPLASH_DURATION_MILLIS도 상수로 잘 정의되어 있습니다.

2. Jetpack Compose 코드

  • Composable 함수의 불필요한 recomposition 유발 여부 (remember, key 사용)
    • remember, rememberSaveable, collectAsStateWithLifecycle 등을 사용하여 UI 상태 변화에 따른 불필요한 리컴포지션을 최소화하려 노력한 흔적이 보입니다. 좋습니다.
    • MyPageScreen에서 다이얼로그의 onConfirm, onDismiss 람다에서 state.authStatus != LoadStatus.Loading을 체크하여 중복 액션을 막은 것은 좋은 사용자 경험 개선이자 잠재적 버그 방지입니다.
  • State hoisting 원칙 준수
    • MyPageEntryBuilder에서 MyPageScreen으로 onNavigateToLogin 람다를 전달하는 등, Composable 간의 상태 호이스팅 원칙을 잘 준수하고 있습니다.
  • side effect (LaunchedEffect, DisposableEffect) 사용의 적절성
    • collectSideEffect를 사용하여 MyPageSideEffect, OnboardingSideEffect 등 ViewModel에서 발생하는 일회성 이벤트를 UI에서 적절하게 처리하고 있습니다. 특히 Toast 메시지 출력이나 내비게이션 처리에 collectSideEffect를 사용한 점이 좋습니다. LaunchedEffectDisposableEffect는 이번 diff에는 사용되지 않았습니다.
  • UI 상태를 data class로 표현했는지
    • MyPageState, Onboarding1State, Onboarding2State 등 UI 계층의 상태를 data class로 명확하게 표현하고 있습니다.
  • 로딩/에러 상태를 Boolean 대신 LoadStatus로 관리하는지
    • MyPageStateOnboardingStateLoadStatus를 사용하여 로딩/에러 상태를 관리하고 있습니다. 매우 좋은 접근 방식이며, MyPageScreen에서 isLoading 상태에 따라 버튼 활성화 여부를 제어하는 등 UI에 반영하는 방식도 올바릅니다.

3. Repository/DataSource 레이어

  • Retrofit 에러 핸들링 (try-catch, Result 래핑)
    • LoginRepositoryImpl, MyPageRepositoryImpl, OnboardingRepositoryImpl에서 safeApiResult, safeApiResultUnit 유틸리티 함수를 사용하여 Retrofit API 호출 결과에 대해 Result 타입을 반환하고 있습니다. 이 접근 방식은 에러를 상위 계층으로 전파하고 ViewModel에서 일관되게 처리할 수 있도록 해주므로 매우 효과적입니다. 좋습니다.
  • 네트워크 응답과 도메인 모델 매핑 분리 여부
    • OnboardingRepositoryImpl에서 MemberOnboardingInput, CharacterOnboardingInput 도메인 모델을 UpdateMemberRequestDto, CreateCharacterRequestDto 네트워크 DTO로 매핑하여 API 요청을 보내고 있습니다. 이는 네트워크 계층과 도메인 계층의 분리를 잘 지킨 예시입니다.
  • 캐싱 전략 (로컬 DB vs 메모리)
    • TokenDataStore에서 불필요한 메모리 캐싱(AtomicReference) 로직을 제거하고 DataStore를 통해 직접 토큰을 관리하도록 변경한 점이 긍정적입니다. AuthInterceptorTokenAuthenticator에서 runBlocking을 사용하여 DataStore의 최신 값을 읽어오는 방식은 이 특정 컨텍스트에서 합리적인 선택입니다.
    • 개선 필요: MyPageRepositoryImpl.ktgetMyProfile() 함수가 여전히 delay(500)과 하드코딩된 데이터를 반환하고 있습니다. PR 설명에는 Repository 구현체가 "실제로 서버 통신을 수행"하도록 변경되었다고 되어 있으나, 이 부분은 아직 임시 구현체입니다. 실제 API 연동이 필요합니다.

4. ViewModel

  • UI 상태와 비즈니스 로직 분리
    • MyPageViewModel, OnboardingViewModel 모두 MyPageState, OnboardingState를 통해 UI 상태를 관리하고, intent 함수 내에서 비즈니스 로직(로그아웃, 탈퇴, 온보딩 제출)을 처리하며 reduce를 통해 상태를 변경합니다. 명확한 분리를 보여줍니다.
  • Orbit ContainerHost 패턴 준수 여부
    • ContainerHost, intent, reduce, postSideEffect 등 Orbit MVI 패턴을 충실히 준수하고 있습니다.
  • intent 내부에서 상태 변경(reduce)만 수행하는지
    • intent 내부에서 reduce를 통해 상태를 변경하고, postSideEffect를 통해 일회성 이벤트를 발생시키는 패턴을 잘 따르고 있습니다. 비동기 작업의 로딩 상태도 reduce를 통해 업데이트하고 있습니다.
  • postSideEffect 남용 여부
    • 내비게이션, 토스트 메시지 등 일회성 이벤트에 postSideEffect를 적절하게 사용하고 있습니다. 남용으로 보이지 않습니다.
  • blocking 작업을 intent 내부에서 직접 수행하지 않는지
    • 네트워크 호출과 같은 잠재적 blocking 작업은 suspend 함수로 Repository에 위임하고, ViewModel에서는 이들을 호출하는 방식으로 처리하고 있습니다. LoadStatus를 통해 UI에서 blocking 상태를 인지할 수 있도록 한 것도 좋습니다.

5. FCM/SSE/실시간 통신 관련 코드

  • 연결 재시도 로직 (exponential backoff)
    • 이번 PR 범위에 직접적으로 FCM/SSE 코드는 포함되어 있지 않습니다.
  • 백그라운드 제약 (Doze 모드, 배터리 최적화) 대응 여부
    • 직접적인 대응 코드는 없지만, TokenAuthenticator에서 401 에러 발생 시 AuthSessionManager.notifySessionExpired()를 호출하여 AppViewModel을 통해 NavigateToLogin을 유발하는 메커니즘은 앱의 인증 상태 변화에 따라 FCM/SSE 등의 연결을 적절히 재설정할 수 있는 기반을 마련해줍니다. 이는 간접적으로 백그라운드에서의 인증 문제에 대응할 수 있는 좋은 아키텍처입니다.
  • 리소스 정리 (연결 해제, 리스너 해제)
    • 로그아웃 및 회원 탈퇴 시 TokenDataStore.clearTokens()를 호출하여 토큰 데이터를 제거하는 것은 중요한 리소스 정리 작업입니다. KakaoLogoutManager를 통해 카카오 SDK 로그아웃도 처리하는 점도 좋습니다.

전반적인 개선 제안

  1. MyPageRepositoryImpl.ktgetMyProfile() 실제 API 연동: 가장 시급한 개선 사항입니다. 현재 임시 구현체는 실제 서비스 로직과 동떨어져 있으며, 테스트 및 배포 시 문제를 일으킬 수 있습니다.
  2. 하드코딩된 "Coming Soon" 텍스트: MyPageScreen에서 showComingSoon 상태를 표시하는 경우, 이 텍스트를 stringResource로 관리하는 것이 좋습니다.
  3. Onboarding6ScreenisLoading prop: OnboardingEntryBuilder에서 isLoading = false로 고정되어 있습니다. 만약 Onboarding6Screen에서도 로딩 상태가 필요하다면 ViewModel에서 이를 관리하고 전달해야 합니다. 현재로서는 불필요한 매개변수이거나, 향후 확장을 위한 미리보기로 보입니다.
  4. data classequals()hashCode() 최적화: OnboardingInput 모델이나 DTOs에서 List<CharacterTraitInput> 또는 List<CharacterTraitRequestDto>와 같이 리스트를 포함하는 경우, 기본 equals/hashCode 구현이 리스트의 요소 순서까지 비교합니다. 만약 순서가 중요하지 않다면 Set으로 변경하거나 equals/hashCode를 오버라이드하여 최적화할 수 있습니다. (현재는 큰 문제는 아닙니다.)

결론

이번 PR은 전반적으로 매우 높은 수준의 코드 품질과 아키텍처 준수율을 보여줍니다. 특히 Orbit MVI 패턴의 올바른 사용, LoadStatus를 통한 상태 관리, Result 타입을 활용한 에러 처리, 그리고 TokenAuthenticatorAuthSessionManager를 연동하여 전역적인 세션 만료 처리를 구현한 점은 훌륭합니다. 위에 언급된 MyPageRepositoryImpl의 임시 구현체 문제를 해결하면 바로 머지해도 좋을 것 같습니다.

계속해서 좋은 코드 부탁드립니다!

@JiwonLee42 JiwonLee42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ViewModel 코드 나중에 꼭 수정하기!!

@JiwonLee42
JiwonLee42 merged commit 0bff824 into develop Jul 31, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] 로그인화면 api연결

3 participants