-
Notifications
You must be signed in to change notification settings - Fork 1
[REF/#682] Abstract device info retrieval and improve data module structure #683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c37f0b4
4a2bcbf
8b6a1f7
1abfb98
09a0dfb
9d25d32
e192644
b53f8a8
9acb62c
415271e
e62019c
d623a18
65d385b
cb02600
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package com.hilingual.app | ||
|
|
||
| import android.content.Context | ||
| import android.os.Build | ||
| import com.hilingual.core.common.extension.appVersionName | ||
| import com.hilingual.core.common.app.DeviceInfoProvider | ||
| import dagger.hilt.android.qualifiers.ApplicationContext | ||
| import javax.inject.Inject | ||
|
|
||
| internal class DeviceInfoProviderImpl @Inject constructor( | ||
| @ApplicationContext private val context: Context | ||
| ) : DeviceInfoProvider { | ||
|
|
||
| override fun getDeviceName(): String = Build.MODEL | ||
|
|
||
| override fun getDeviceType(): String = | ||
| if (context.resources.configuration.smallestScreenWidthDp >= 600) "TABLET" else "PHONE" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이전에도 있었던 코드이긴 하지만, 이번에 보다가 궁금한 점이 생겨서 댓글 남깁니다..! smallestscreenWidthDp에 따라 디바이스 타입이 결정되는 것으로 이해했는데요, 코드를 보니 로그인 시점에만 이 부분이 호출되는 것으로 파악했습니다. 이 값이 API 호출 시 단순 통계/로그용으로만 사용되는 것인지 궁금합니다!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 앱에서는 넓이를 기준으로 변하는 디자인을 채용하고 있지 않습니다. 해당 정보는 서비스 서버의 응답을 위한 코드입니다. 노션에 명세를 보면 알수있어요👍🏻 |
||
|
|
||
| override fun getOsVersion(): String = Build.VERSION.RELEASE | ||
|
|
||
| override fun getAppVersion(): String = context.appVersionName | ||
|
|
||
| override fun getProvider(): String = "GOOGLE" | ||
|
|
||
| override fun getRole(): String = "USER" | ||
|
|
||
| override fun getOsType(): String = "Android" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package com.hilingual.di | ||
|
|
||
| import com.hilingual.core.common.app.DeviceInfoProvider | ||
| import com.hilingual.app.DeviceInfoProviderImpl | ||
| import dagger.Binds | ||
| import dagger.Module | ||
| import dagger.hilt.InstallIn | ||
| import dagger.hilt.components.SingletonComponent | ||
| import javax.inject.Singleton | ||
|
|
||
| @Module | ||
| @InstallIn(SingletonComponent::class) | ||
| internal abstract class ProviderModule { | ||
|
|
||
| @Binds | ||
| @Singleton | ||
| abstract fun bindDeviceInfoProvider( | ||
| impl: DeviceInfoProviderImpl | ||
| ): DeviceInfoProvider | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| package com.hilingual.core.common.constant | ||
|
|
||
| const val STABLE_VERSION = "2.0.0" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 버전 관련 상수를 모아두는 용도로
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 기존 DEFAULT_VERSION이라는 이름은 '0.0.0' 같은 단순히 빈 값을 채우는 의미로만 느껴져서, 앱이 정상적으로 동작한다고 보장할 수 있는 최소한의 기준점을 명시하고자 STABLE_VERSION으로 변경했습니다. 그리고 버전 추출 실패 시 비정상적인 버전 값 때문에 강제 업데이트 로직이 잘못 실행되는 등의 부수 효과를 방지하는 안전장치 역할을 합니다. 앞으로는 MIN_SUPPORTED_VERSION처럼 정책적인 상수들이 추가될 수 있을 것 같아요.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 앞으로 버전 계속 올라갈텐데 STABLE_VERSION은 고정인가요? 아님 메이저만 반영하는 등의 기준이 있을까요?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 값은 고정이라기보다는 프로젝트의 기준이 되는 '최소 보장 버전'의 성격입니다. 버전 정보를 가져오지 못하는 예외 상황에서도 앱의 비즈니스 로직이 깨지지 않게 하기 위한 최소한의 가이드라인이라고 봐주시면 될 것 같습니다. 말씀하신 대로 메이저 업데이트나 중요한 정책 변경이 있을 때 이 상수도 함께 관리할 예정입니다! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,25 @@ | |
| package com.hilingual.core.common.extension | ||
|
|
||
| import android.content.Context | ||
| import android.content.pm.PackageManager | ||
| import android.os.Build | ||
| import androidx.browser.customtabs.CustomTabsIntent | ||
| import androidx.core.net.toUri | ||
| import com.hilingual.core.common.constant.STABLE_VERSION | ||
|
|
||
| fun Context.launchCustomTabs(url: String) { | ||
| CustomTabsIntent.Builder().build().launchUrl(this, url.toUri()) | ||
| } | ||
|
|
||
| val Context.appVersionName: String | ||
| get() = runCatching { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기에서
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. appVersionName을 확장 프로퍼티로 선언했기 때문에 호출하는 쪽에서 별도의 코루틴 스코프 없이도 동기적으로 값을 바로 얻을 수 있도록 runCatching을 사용했습니다. |
||
| val packageInfo = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
| packageManager.getPackageInfo( | ||
| packageName, | ||
| PackageManager.PackageInfoFlags.of(0) | ||
| ) | ||
| } else { | ||
| packageManager.getPackageInfo(packageName, 0) | ||
| } | ||
| packageInfo.versionName | ||
| }.getOrNull() ?: STABLE_VERSION | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 음 버전을 UI에서 불러올때 사용하는걸 의도했습니다. 정말 단순하게 버전을 불러올 뿐이라면 해당 함수를 사용하는게 더 간결하니까요! 해당 의도를 더 전달하기 위해서 Mypage는 Provider없이 사용하도록 수정하겠습니다 |
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,12 @@ | |
| package com.hilingual.data.auth.repositoryimpl | ||
|
|
||
| import android.content.Context | ||
| import com.hilingual.core.common.app.DeviceInfoProvider | ||
| import com.hilingual.core.common.util.suspendRunCatching | ||
| import com.hilingual.core.localstorage.TokenManager | ||
| import com.hilingual.core.localstorage.UserInfoManager | ||
| import com.hilingual.data.auth.datasource.AuthRemoteDataSource | ||
| import com.hilingual.data.auth.datasource.GoogleAuthDataSource | ||
| import com.hilingual.data.auth.datasource.SystemDataSource | ||
| import com.hilingual.data.auth.dto.request.LoginRequestDto | ||
| import com.hilingual.data.auth.dto.request.VerifyCodeRequestDto | ||
| import com.hilingual.data.auth.model.LoginModel | ||
|
|
@@ -31,7 +31,7 @@ import javax.inject.Inject | |
| internal class AuthRepositoryImpl @Inject constructor( | ||
| private val authRemoteDataSource: AuthRemoteDataSource, | ||
| private val googleAuthDataSource: GoogleAuthDataSource, | ||
| private val systemDataSource: SystemDataSource, | ||
| private val deviceInfoProvider: DeviceInfoProvider, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Manager과 Provider 명칭을 구분하시는 기준이 있으실까요??
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Manager와 Provider 명칭에 대해 저도 고민을 많이 했었는데요! 제가 생각하는 기준은 '상태 관리나 저장/수정 로직이 포함되는가'입니다. TokenManager나 UserInfoManager는 데이터를 로컬에 저장하고 갱신하거나 삭제하는 등 데이터의 생명주기를 CRUD하는 성격이 강합니다. |
||
| private val tokenManager: TokenManager, | ||
| private val userInfoManager: UserInfoManager | ||
| ) : AuthRepository { | ||
|
|
@@ -41,7 +41,7 @@ internal class AuthRepositoryImpl @Inject constructor( | |
| override suspend fun login(providerToken: String): Result<LoginModel> = suspendRunCatching { | ||
| val loginResponse = authRemoteDataSource.login( | ||
| providerToken = providerToken, | ||
| loginRequestDto = systemDataSource.toLoginRequestDto() | ||
| loginRequestDto = deviceInfoProvider.toLoginRequestDto() | ||
| ).data!! | ||
|
|
||
| tokenManager.saveTokens(loginResponse.accessToken, loginResponse.refreshToken) | ||
|
|
@@ -70,7 +70,7 @@ internal class AuthRepositoryImpl @Inject constructor( | |
| userInfoManager.clear() | ||
| } | ||
|
|
||
| private fun SystemDataSource.toLoginRequestDto() = LoginRequestDto( | ||
| private fun DeviceInfoProvider.toLoginRequestDto() = LoginRequestDto( | ||
| provider = this.getProvider(), | ||
| role = this.getRole(), | ||
| deviceName = this.getDeviceName(), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package com.hilingual.data.config.di | ||
|
|
||
| import com.hilingual.data.config.repository.ConfigRepository | ||
| import com.hilingual.data.config.repositoryimpl.ConfigRepositoryImpl | ||
| import dagger.Binds | ||
| import dagger.Module | ||
| import dagger.hilt.InstallIn | ||
| import dagger.hilt.components.SingletonComponent | ||
| import javax.inject.Singleton | ||
|
|
||
| @Module | ||
| @InstallIn(SingletonComponent::class) | ||
| internal abstract class RepositoryModule { | ||
|
|
||
| @Binds | ||
| @Singleton | ||
| abstract fun bindConfigRepository( | ||
| impl: ConfigRepositoryImpl | ||
| ): ConfigRepository | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.