NR-160 일부 기기에서 구글 로그인이 불가능한 이슈

[juhwankim-dev]

예전에는 data에서 구글로그인을 처리하고 있었음

그러다보니 getCredentials에서 applicationContext를 넘겼었고 이때문에 OS 13 이하 버전에서 구글 로그인에 문제가 발생했음

activity를 넘기기 위해 presentation 쪽으로 옮김

[github-actions]

🤖 AI Code Review (Gemini)

NextRoom 프로젝트의 Pull Request에 대한 코드 리뷰를 시작하겠습니다.

PR 설명에 따르면, 구글 로그인 시 applicationContext 사용으로 인해 Android 13 이하 버전에서 발생하던 문제를 해결하기 위해, Activity 컨텍스트가 필요한 구글 로그인 로직을 data 레이어에서 presentation 레이어로 이동했습니다. 이 변경 사항은 클린 아키텍처 원칙에 부합하며, UI 관련 로직이 presentation 레이어에 위치해야 한다는 점에서 매우 긍정적입니다.

전반적으로 변경 의도는 명확하고, 대부분의 파일에서 data 레이어의 불필요한 의존성을 제거하고 presentation 레이어로 이동하는 작업은 잘 수행되었습니다. 특히 ViewModel과 Fragment 간의 역할 분담이 명확해진 점은 좋습니다.

하지만 몇 가지 중요한 문제점이 발견되었습니다.

---

🔴 Critical (치명적)

1. presentation/build.gradle.kts에 누락된 의존성

   • 파일: presentation/build.gradle.kts
   • 문제: data/build.gradle.kts에서 credentials, credentials.auth, google.identity 라이브러리 의존성이 제거되었으나, presentation/build.gradle.kts에는 추가되지 않았습니다. GoogleAuthClient 클래스가 이 라이브러리들을 사용하므로, 의존성이 없으면 빌드 실패 또는 런타임 크래시가 발생할 것입니다.
   • 해결 방안: presentation/build.gradle.kts 파일에 다음 의존성을 추가해야 합니다.

     // Google Identity
     implementation(libs.credentials)
     implementation(libs.credentials.auth)
     implementation(libs.google.identity)

2. GoogleAuthClient.kt에서 CredentialManager 초기화 문제

   • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/util/GoogleAuthClient.kt
   • 문제: GoogleAuthClient는 @Singleton으로 선언되어 ApplicationContext를 주입받아 CredentialManager.create(context)를 호출합니다. 그러나 CredentialManager 공식 문서에 따르면, 계정 선택 UI를 띄우려면 CredentialManager.create(Context) 호출 시 Activity 컨텍스트를 사용해야 합니다. ApplicationContext로 CredentialManager를 생성하면 PR의 목표(Android 13 이하 버전에서 발생하는 문제 해결)를 달성하지 못하고 동일한 문제가 재발할 가능성이 매우 높습니다.
   • 해결 방안:
     • GoogleAuthClient를 싱글톤으로 만들지 않고, Fragment 내에서 CredentialManager.create(requireActivity())를 호출하여 CredentialManager 인스턴스를 생성한 후, 이 인스턴스를 GoogleAuthClient의 생성자나 메서드로 전달하는 방식으로 변경하는 것이 가장 안전합니다.
     • 예시:

       // presentation/src/main/java/com/nextroom/nextroom/presentation/util/GoogleAuthClient.kt
       // @Singleton 어노테이션 제거
       // @ApplicationContext context: Context 제거
       class GoogleAuthClient @Inject constructor() { // Hilt로 주입받을 다른 의존성이 있다면 여기에 추가
           private val request: GetCredentialRequest by lazy {
               // ... 기존 로직 유지
           }
       
           // CredentialManager를 외부에서 주입받도록 변경
           suspend fun requestGoogleIdToken(activity: Activity, credentialManager: CredentialManager): String {
               val response = credentialManager.getCredential(
                   context = activity,
                   request = request,
               )
               return response.extractIdToken()
           }
           // ... 기존 로직 유지
       }
       
       // Fragment (EmailLoginFragment, LoginFragment)
       // @Inject lateinit var googleAuthClient: GoogleAuthClient
       private fun requestGoogleAuth() {
           viewLifecycleOwner.lifecycleScope.launch {
               val credentialManager = CredentialManager.create(requireActivity()) // Activity Context로 생성
               val idToken = try {
                   googleAuthClient.requestGoogleIdToken(requireActivity(), credentialManager)
               } catch (e: Exception) {
                   // ... 기존 로직 유지
               }
               // ... 기존 로직 유지
           }
       }

     • 또는 GoogleAuthClient를 Activity 스코프로 만들고 Activity를 주입받는 방법도 고려할 수 있으나, Hilt의 기본 스코프만으로는 어려울 수 있습니다. 현재 구조에서는 Fragment에서 CredentialManager를 생성하여 전달하는 방식이 가장 간단하고 명확합니다.

---

🟡 Warning (경고)

1. 일반적인 오류 메시지
   • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/ui/login/EmailLoginFragment.kt (Line 50)
   • 문제: 구글 인증 과정에서 예상치 못한 Exception 발생 시 toast(R.string.error_something)와 같은 일반적인 오류 메시지를 표시합니다. 사용자 경험 측면에서 가능한 경우 더 구체적인 오류 메시지를 제공하는 것이 좋습니다.
   • 해결 방안: 모든 예외를 상세하게 처리하기는 어렵겠지만, 만약 특정 예외 상황(예: 네트워크 연결 끊김 등)에 대해 더 명확한 사용자 메시지를 제공할 수 있다면 개선을 고려해볼 수 있습니다. 현재로서는 예상치 못한 오류에 대한 일반적인 처리로 허용 가능합니다.

---

💡 Suggestion (제안)

1. GoogleAuthClient의 serverClientId 관리
   • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/util/GoogleAuthClient.kt (Line 24)
   • 문제: GetGoogleIdOption.Builder().setServerClientId(BuildConfig.O_AUTH_WEB_CLIENT_ID)를 GoogleAuthClient 내에서 직접 참조하고 있습니다. GoogleAuthClient가 CredentialManager 초기화 문제로 인해 싱글톤이 아니게 된다면, request 객체도 매번 생성될 수 있습니다. serverClientId는 변경되지 않는 값이므로, 이를 GoogleAuthClient의 생성자를 통해 주입받거나, request 객체를 lazy로 초기화하는 것은 괜찮지만, BuildConfig에 직접 의존하는 대신 Hilt를 통해 주입받는 방식도 고려해볼 수 있습니다.
   • 해결 방안: GoogleAuthClient가 String 타입의 serverClientId를 생성자를 통해 주입받도록 하고, presentation 모듈의 Hilt Module에서 BuildConfig.O_AUTH_WEB_CLIENT_ID를 @Provides로 제공하는 방식을 고려해볼 수 있습니다. 이렇게 하면 GoogleAuthClient가 BuildConfig에 직접 의존하지 않게 되어 테스트 용이성이 향상됩니다.

---

긍정적인 측면 (Positive Aspects)

• 클린 아키텍처 준수: Activity 컨텍스트가 필요한 구글 로그인 로직을 data 레이어에서 presentation 레이어로 성공적으로 이동하여 클린 아키텍처 원칙을 잘 준수했습니다. 이는 UI 관련 로직이 presentation 레이어에 위치해야 한다는 점에서 매우 바람직합니다.
• 명확한 관심사 분리: ViewModel은 idToken을 이용한 로그인 비즈니스 로직에 집중하고, Fragment는 Activity 컨텍스트를 사용하여 idToken을 얻는 UI 상호작용을 담당하도록 역할이 명확하게 분리되었습니다.
• 생명주기 인식: Fragment에서 viewLifecycleOwner.lifecycleScope.launch를 사용하여 코루틴을 실행함으로써 안드로이드 컴포넌트의 생명주기를 안전하게 따르고 있습니다.
• 견고한 오류 처리: 사용자가 계정 선택을 취소했을 때 발생하는 GetCredentialCancellationException을 명시적으로 처리하여 불필요한 오류 메시지를 방지한 점이 좋습니다.
• 일관된 패턴: EmailLoginFragment/EmailLoginViewModel과 LoginFragment/LoginViewModel에 걸쳐 변경 사항이 일관된 패턴으로 적용되었습니다.
• 로딩 상태 관리: 계정 선택 UI가 뜨기 전부터 로딩 상태를 표시하고, 성공/실패 시 적절히 해제하는 로딩 상태 관리가 잘 구현되었습니다.

---

종합 의견

PR의 의도는 매우 좋고, 클린 아키텍처를 향한 올바른 방향의 변경입니다. data 레이어에서 UI 관련 의존성을 제거하고 presentation 레이어로 옮긴 것은 훌륭합니다.

하지만 presentation/build.gradle.kts에 필수적인 라이브러리 의존성이 누락되어 있고, GoogleAuthClient에서 CredentialManager를 ApplicationContext로 초기화하는 치명적인 문제가 있습니다. 이 두 가지 문제를 해결하지 않으면 PR의 핵심 목표를 달성할 수 없거나 빌드 자체가 불가능할 것입니다.

위의 Critical 문제들을 우선적으로 해결한 후, Warning 및 Suggestion 사항들을 고려하여 개선하면 더욱 완성도 높은 코드가 될 것입니다.

---

terminal-notifier -title "Claude Code" -message "코드 리뷰가 완료되었습니다. Critical 이슈를 확인해주세요." -sound default

---

This review was automatically generated using Gemini AI. Please use your judgment when addressing the feedback.

[juhwankim-dev]

1번은 ai가 잘못 분석한듯 의존성 들어가있음
2번은 get 경로에서는 create(context)로 넘긴 Context가 아예 쓰이지 않음. 문제없음