[Feat/#238] FireBase RemoteConfig를 통한 앱 버전 관리를 구현합니다.#247
[Feat/#238] FireBase RemoteConfig를 통한 앱 버전 관리를 구현합니다.#247MoonsuKang merged 11 commits intodevelopfrom
Conversation
WalkthroughThe changes integrate Firebase Remote Config for app version management. The update includes dependency and configuration modifications in Gradle files, along with new classes and interfaces to check for app updates. A dedicated remote config data source is introduced to retrieve version data. The app update checker (with soft/hard update differentiation) is injected via a new Dagger module and integrated into the splash screen UI with corresponding dialogs. Additionally, utility functions managing market redirection were added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SplashScreen
participant ViewModel
participant AppUpdateChecker
participant RemoteConfigDataSource
participant FirebaseRemoteConfig
User->>SplashScreen: Launch App
SplashScreen->>ViewModel: Start & collect updateState
ViewModel->>AppUpdateChecker: getAppUpdateState(currentVersion)
AppUpdateChecker->>RemoteConfigDataSource: fetch(), getLatestVersion(), getMinimumVersion()
RemoteConfigDataSource->>FirebaseRemoteConfig: fetchAndActivate()
FirebaseRemoteConfig-->>RemoteConfigDataSource: Config Data
RemoteConfigDataSource-->>AppUpdateChecker: Return versions
AppUpdateChecker-->>ViewModel: Return update state (Latest, SoftUpdate, HardUpdate)
ViewModel-->>SplashScreen: Provide updateState
SplashScreen->>User: Show dialog if update required
sequenceDiagram
participant User
participant SplashScreen
participant AppUpdateUtils
participant PlayStore
User->>SplashScreen: Chooses to update (soft/hard)
SplashScreen->>AppUpdateUtils: navigateToMarketAndFinish(activity)
AppUpdateUtils->>PlayStore: Launch market intent
PlayStore-->>AppUpdateUtils: Opened Play Store
AppUpdateUtils->>SplashScreen: Close app
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
app/src/main/java/com/sopt/clody/domain/util/VersionComparator.kt (1)
3-16: Version comparison logic works but could be improved.The implementation correctly compares semantic version strings, handling different version lengths and non-numeric parts appropriately. However, consider these improvements:
- Add documentation to explain the return values (-1, 0, 1)
- Add input validation for empty or malformed version strings
- Consider handling pre-release versions (like 1.0.0-beta)
object VersionComparator { + /** + * Compares two version strings in the format "x.y.z". + * @param current The current version string + * @param latest The latest version string to compare against + * @return -1 if current < latest, 0 if equal, 1 if current > latest + */ fun compare(current: String, latest: String): Int { + if (current.isEmpty() || latest.isEmpty()) { + throw IllegalArgumentException("Version strings cannot be empty") + } + val currentParts = current.split(".").map { it.toIntOrNull() ?: 0 } val latestParts = latest.split(".").map { it.toIntOrNull() ?: 0 } for (i in 0 until maxOf(currentParts.size, latestParts.size)) { val c = currentParts.getOrNull(i) ?: 0 val l = latestParts.getOrNull(i) ?: 0 if (c < l) return -1 if (c > l) return 1 } return 0 } }app/src/main/java/com/sopt/clody/presentation/utils/appupdate/AppUpdateUtils.kt (1)
14-28: Well-implemented market navigation with fallback logicThe navigation logic correctly attempts to use the direct market URI first and gracefully falls back to a web URL when the market app isn't available. The use of
FLAG_ACTIVITY_NEW_TASKis appropriate for launching an external activity.Consider adding try-catch blocks to handle potential exceptions when starting activities, as this could fail in rare cases.
fun navigateToMarket(context: Context) { val packageName = context.packageName val marketIntent = Intent(Intent.ACTION_VIEW, Uri.parse("market://details?id=$packageName")).apply { addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) } if (marketIntent.resolveActivity(context.packageManager) != null) { + try { context.startActivity(marketIntent) + } catch (e: Exception) { + val webIntent = Intent(Intent.ACTION_VIEW, Uri.parse("https://play.google.com/store/apps/details?id=$packageName")).apply { + addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + } + context.startActivity(webIntent) + } } else { val webIntent = Intent(Intent.ACTION_VIEW, Uri.parse("https://play.google.com/store/apps/details?id=$packageName")).apply { addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) } + try { context.startActivity(webIntent) + } catch (e: Exception) { + // Log the error or show a toast message + } } }app/src/main/java/com/sopt/clody/domain/model/AppUpdateState.kt (1)
3-7: Effective sealed interface for app update statesThe sealed interface is a good choice for modeling mutually exclusive update states. The use of data object for Latest and data classes for update states with version information is clean and follows Kotlin best practices.
Consider adding more metadata to update states, such as update messages or changelog information that could be shown to users.
sealed interface AppUpdateState { data object Latest : AppUpdateState - data class SoftUpdate(val latestVersion: String) : AppUpdateState - data class HardUpdate(val latestVersion: String) : AppUpdateState + data class SoftUpdate( + val latestVersion: String, + val updateMessage: String = "", + val changelogUrl: String? = null + ) : AppUpdateState + data class HardUpdate( + val latestVersion: String, + val updateMessage: String = "", + val changelogUrl: String? = null + ) : AppUpdateState }app/src/main/java/com/sopt/clody/presentation/ui/splash/SplashScreen.kt (2)
97-115: Well-implemented SoftUpdateDialog composable.The dialog provides clear information about the optional update with appropriate actions (update now or later).
Consider adding more context in the dialog text explaining what improvements the new version brings, if this information is available from RemoteConfig.
124-124: Consider handling back button for hard update dialog.The empty onDismissRequest works, but explicitly handling back button presses with the same action as "앱 종료" would make the behavior more explicit.
- onDismissRequest = {}, + onDismissRequest = onExit,app/src/main/java/com/sopt/clody/data/remote/datasource/RemoteConfigDataSource.kt (1)
16-17: Config keys should be defined as constants.The string literals "latest_version" and "min_required_version" should be extracted to constants to avoid magic strings and make maintenance easier.
+ companion object { + private const val KEY_LATEST_VERSION = "latest_version" + private const val KEY_MINIMUM_VERSION = "min_required_version" + } - fun getLatestVersion(): String = remoteConfig.getString("latest_version") - fun getMinimumVersion(): String = remoteConfig.getString("min_required_version") + fun getLatestVersion(): String = remoteConfig.getString(KEY_LATEST_VERSION) + fun getMinimumVersion(): String = remoteConfig.getString(KEY_MINIMUM_VERSION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/build.gradle.kts(2 hunks)app/src/main/java/com/sopt/clody/data/remote/appupdate/AppUpdateCheckerImpl.kt(1 hunks)app/src/main/java/com/sopt/clody/data/remote/datasource/RemoteConfigDataSource.kt(1 hunks)app/src/main/java/com/sopt/clody/di/AppUpdateModule.kt(1 hunks)app/src/main/java/com/sopt/clody/domain/appupdate/AppUpdateChecker.kt(1 hunks)app/src/main/java/com/sopt/clody/domain/model/AppUpdateState.kt(1 hunks)app/src/main/java/com/sopt/clody/domain/util/VersionComparator.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/splash/SplashScreen.kt(4 hunks)app/src/main/java/com/sopt/clody/presentation/ui/splash/SplashViewModel.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/utils/appupdate/AppUpdateUtils.kt(1 hunks)gradle/libs.versions.toml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (19)
app/src/main/java/com/sopt/clody/domain/appupdate/AppUpdateChecker.kt (1)
1-7: Interface looks good!The
AppUpdateCheckerinterface is well-designed with a clear single responsibility of determining the app's update state. The suspend function indicates proper handling of asynchronous operations, which is appropriate for fetching remote configuration data.app/build.gradle.kts (2)
29-29: Version name increment is appropriate.Incrementing the version name from 1.0.6 to 1.0.7 aligns with the implementation of the new app version management feature.
102-102: Firebase Remote Config dependency added correctly.The Firebase Remote Config KTX library has been correctly added to support the app version management functionality described in the PR objectives.
gradle/libs.versions.toml (2)
49-49: Firebase Remote Config version specified correctly.The Firebase Remote Config KTX library version is properly defined.
102-102: Firebase Remote Config library entry added properly.The Firebase Remote Config KTX library entry is correctly added with appropriate group and name specifications.
app/src/main/java/com/sopt/clody/presentation/utils/appupdate/AppUpdateUtils.kt (1)
34-37: Appropriate implementation for hard update scenarioThe implementation correctly directs users to the app store and then closes the app using
finishAffinity(), which is the right approach for a hard update scenario.Consider adding a comment explaining that this is an intentional app termination after directing the user to update.
app/src/main/java/com/sopt/clody/di/AppUpdateModule.kt (1)
22-26: AppUpdateChecker dependency is properly injectedThe provider for AppUpdateChecker is correctly implemented using constructor injection for the RemoteConfigDataSource dependency.
app/src/main/java/com/sopt/clody/presentation/ui/splash/SplashViewModel.kt (6)
4-7: Appropriate imports added for app update functionality.The imports are well-organized and aligned with the new app update checking functionality being implemented.
Also applies to: 12-12
18-18: Good use of dependency injection for AppUpdateChecker.The AppUpdateChecker is properly injected into the ViewModel, following dependency injection best practices.
24-25: State management for update status is well implemented.The update state is properly encapsulated in a private MutableStateFlow with a public immutable StateFlow for observers, following the recommended pattern for state management in ViewModels.
29-29: Version check is appropriately triggered in init block.Calling checkVersion() during initialization ensures the app update status is checked as soon as the ViewModel is created.
38-43: Well-structured version checking logic.The version checking logic appropriately uses coroutines via viewModelScope to perform the asynchronous operation. Good practice passing the current app version from BuildConfig.
45-47: Good implementation of clearUpdateState method.This method allows resetting the update state which is useful when a user chooses to dismiss a soft update notification.
app/src/main/java/com/sopt/clody/presentation/ui/splash/SplashScreen.kt (4)
38-40: Good state collection and context acquisition.The update state is properly collected using collectAsStateWithLifecycle() and the activity context is retrieved appropriately for navigating to the app store.
42-55: Enhanced LaunchedEffect with proper navigation logic.The LaunchedEffect now correctly depends on both login state and update state, ensuring navigation only occurs when the app is up-to-date.
57-75: Well-structured update state handling with proper UI responses.The when statement elegantly handles different update states, showing appropriate dialogs based on whether the update is soft or hard.
117-130: Well-implemented HardUpdateDialog with forced update pattern.The dialog correctly implements a forced update pattern with no ability to dismiss via back button (empty onDismissRequest). The options are clear: update or exit.
app/src/main/java/com/sopt/clody/data/remote/datasource/RemoteConfigDataSource.kt (2)
8-11: Well-structured RemoteConfigDataSource with proper DI.The class is correctly annotated with @singleton and uses constructor injection for FirebaseRemoteConfig, following best practices for dependency injection.
12-14: Good use of suspend function for asynchronous remote config fetching.The fetch() method correctly uses the suspend keyword and await() extension to handle the asynchronous Firebase operation in a coroutine-friendly way.
| @Provides | ||
| @Singleton | ||
| fun provideFirebaseRemoteConfig(): FirebaseRemoteConfig = | ||
| FirebaseRemoteConfig.getInstance() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Firebase RemoteConfig provider should include configuration settings
While the provider correctly returns the Firebase RemoteConfig instance, it's missing the configuration for fetch settings like minimum fetch interval and fetch timeout.
According to the PR description, the fetch interval should be 12 hours, but this isn't set in the code.
@Provides
@Singleton
fun provideFirebaseRemoteConfig(): FirebaseRemoteConfig =
- FirebaseRemoteConfig.getInstance()
+ FirebaseRemoteConfig.getInstance().apply {
+ val configSettings = FirebaseRemoteConfigSettings.Builder()
+ .setMinimumFetchIntervalInSeconds(43200) // 12 hours in seconds
+ .build()
+ setConfigSettingsAsync(configSettings)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Provides | |
| @Singleton | |
| fun provideFirebaseRemoteConfig(): FirebaseRemoteConfig = | |
| FirebaseRemoteConfig.getInstance() | |
| @Provides | |
| @Singleton | |
| fun provideFirebaseRemoteConfig(): FirebaseRemoteConfig = | |
| FirebaseRemoteConfig.getInstance().apply { | |
| val configSettings = FirebaseRemoteConfigSettings.Builder() | |
| .setMinimumFetchIntervalInSeconds(43200) // 12 hours in seconds | |
| .build() | |
| setConfigSettingsAsync(configSettings) | |
| } |
| override suspend fun getAppUpdateState(currentVersion: String): AppUpdateState { | ||
| remoteConfigDataSource.fetch() | ||
|
|
||
| val latestVersion = remoteConfigDataSource.getLatestVersion() | ||
| val minimumVersion = remoteConfigDataSource.getMinimumVersion() | ||
|
|
||
| return when { | ||
| VersionComparator.compare(currentVersion, minimumVersion) < 0 -> { | ||
| AppUpdateState.HardUpdate(latestVersion) | ||
| } | ||
|
|
||
| VersionComparator.compare(currentVersion, latestVersion) < 0 -> { | ||
| AppUpdateState.SoftUpdate(latestVersion) | ||
| } | ||
|
|
||
| else -> { | ||
| AppUpdateState.Latest | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for Remote Config fetching
The implementation correctly determines update states based on version comparison, but lacks error handling for potential failures when fetching remote config.
Since fetching from Firebase might fail due to network issues, you should add try-catch blocks to handle exceptions and implement a fallback strategy.
override suspend fun getAppUpdateState(currentVersion: String): AppUpdateState {
- remoteConfigDataSource.fetch()
+ try {
+ remoteConfigDataSource.fetch()
+ } catch (e: Exception) {
+ // If fetch fails, continue with potentially cached values
+ // Log the error for monitoring
+ }
val latestVersion = remoteConfigDataSource.getLatestVersion()
val minimumVersion = remoteConfigDataSource.getMinimumVersion()
return when {
VersionComparator.compare(currentVersion, minimumVersion) < 0 -> {
AppUpdateState.HardUpdate(latestVersion)
}
VersionComparator.compare(currentVersion, latestVersion) < 0 -> {
AppUpdateState.SoftUpdate(latestVersion)
}
else -> {
AppUpdateState.Latest
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override suspend fun getAppUpdateState(currentVersion: String): AppUpdateState { | |
| remoteConfigDataSource.fetch() | |
| val latestVersion = remoteConfigDataSource.getLatestVersion() | |
| val minimumVersion = remoteConfigDataSource.getMinimumVersion() | |
| return when { | |
| VersionComparator.compare(currentVersion, minimumVersion) < 0 -> { | |
| AppUpdateState.HardUpdate(latestVersion) | |
| } | |
| VersionComparator.compare(currentVersion, latestVersion) < 0 -> { | |
| AppUpdateState.SoftUpdate(latestVersion) | |
| } | |
| else -> { | |
| AppUpdateState.Latest | |
| } | |
| } | |
| } | |
| override suspend fun getAppUpdateState(currentVersion: String): AppUpdateState { | |
| try { | |
| remoteConfigDataSource.fetch() | |
| } catch (e: Exception) { | |
| // If fetch fails, continue with potentially cached values | |
| // Log the error for monitoring | |
| } | |
| val latestVersion = remoteConfigDataSource.getLatestVersion() | |
| val minimumVersion = remoteConfigDataSource.getMinimumVersion() | |
| return when { | |
| VersionComparator.compare(currentVersion, minimumVersion) < 0 -> { | |
| AppUpdateState.HardUpdate(latestVersion) | |
| } | |
| VersionComparator.compare(currentVersion, latestVersion) < 0 -> { | |
| AppUpdateState.SoftUpdate(latestVersion) | |
| } | |
| else -> { | |
| AppUpdateState.Latest | |
| } | |
| } | |
| } |
| override suspend fun getAppUpdateState(currentVersion: String): AppUpdateState { | ||
| remoteConfigDataSource.fetch() | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider optimizing Remote Config fetch frequency
Currently, the implementation calls fetch() every time getAppUpdateState() is called, which might be inefficient if called frequently.
Since Firebase RemoteConfig has a default caching mechanism, consider implementing a more strategic approach to fetching, such as only fetching if the cached data is stale.
Do you have a RemoteConfigDataSource implementation that manages caching efficiently? If not, consider adding a check for the last fetch time before making a new fetch request.
🏁 Script executed:
#!/bin/bash
# Checking if RemoteConfigDataSource has caching logic
rg -A 10 "class RemoteConfigDataSource" --type kotlinLength of output: 1227
Optimize Remote Config Fetching Frequency
The current implementation in AppUpdateCheckerImpl.kt calls remoteConfigDataSource.fetch() on every invocation of getAppUpdateState(). Our investigation of RemoteConfigDataSource.kt confirms that it simply calls fetchAndActivate() without any additional caching logic or a check on the last fetch time. While Firebase RemoteConfig has a built-in caching mechanism, relying solely on it might not be sufficient if this method is hit frequently.
Suggestions:
- Implement an additional caching strategy (e.g., store the last fetch timestamp) and skip fetching if the cached data is still fresh.
- Revisit your caching requirements to determine if Firebase’s default behavior meets your needs or if more fine-grained control is necessary.
app/src/main/java/com/sopt/clody/data/remote/datasource/RemoteConfigDataSource.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/sopt/clody/data/remote/datasource/RemoteConfigDataSource.kt
Show resolved
Hide resolved
- RemoteConfig key 값 companion object로 상수화 - RemoteConfig 값 미존재 시 현재 앱 버전으로 fallback 처리
SYAAINN
left a comment
SYAAINN
left a comment
There was a problem hiding this comment.
코드가 참 이해하기 쉽게 잘 짜여졌네요..
슬슬 멀티모듈의 필요성이 느껴지는 시점에 온 것 같기도 합니다 껄껄,,,
| /** | ||
| * 마켓 이동 | ||
| * @param context Context | ||
| */ |
There was a problem hiding this comment.
[3] 요즘 코드 작성 간에 함수 위에 이렇게 기능과 param에 대한 comment를 남겨놓으시더라구요! 이때까지 솝트에서 프로젝트를 하다보면 리뷰 간에 주석, 로그 제거에 좀 집착(?)하던 경향이 있었는데 어떻게 생각하시나요? 지금정도의 주석은 필요한 주석이다?!
사실 리뷰를 하는 입장에서 제 개인적으로는 좋다고 생각하기는한데 주변에 현직자들의 의견이나 문수님 의견 들은 바가 있다면 공유받고 싶습니다😀
There was a problem hiding this comment.
좋은 질문입니다!
주석 관련 생각?
사실 이 부분은 회사마다 다소 지향하는 바가 다를 수 있는데, 공식적인 KDoc 형식의 주석은 실무에서도 많이 사용하고 권장하는 경우가 많아요. 특히 SDK, Library, Utility Class처럼 다른 모듈이나 팀에서 재사용될 가능성이 있는 클래스나 함수에는 기능과 파라미터 설명을 남기는 게 좋습니다.
BUT, 너무 obvious한 주석(// 클릭 시 이동)이나 개발 도중 찍은 Log는 커밋 전에 정리하는 게 보통 맞습니다.
저도 개인적인 생각으로는 이런 정도의 KDoc 주석은 남기는 게 좋다고 생각하고, 특히 유틸리티성 함수들은 이런 주석이 없으면 오히려 의도가 잘 안 보일 때가 있어서 코드 리뷰할 때도 도움 되는 것 같습니다.
ps: 카카오뱅크에 재직중인 익명의 시니어 분에게 여쭈어 봤는데 이런 형태의 주석을 많이 사용한다고 해요~
주석관련 아티클
그리고 왜 로그를 정리하라고 할까요?
코드리뷰를 받을 때 항상 "불필요한 로그 지워주세요~" 라는 걸 많이 받잖아요? 크게 2가지 이유가 있어요.
첫 번째 - 성능 문제
로그 출력은 단순한 텍스트 출력이 아니라 내부적으로 I/O가 발생해요.
특히 애니메이션, 스크롤, 게임, 실시간 처리가 많은 구간에서 디버그 로그가 남아있으면 프레임 드랍, 렌더링 지연의 원인이 될 수 있답니다?
두 번째 - 불필요한 메모리 할당 및 GC(Garbage Collection) 유발
로그를 위해 생성된 문자열, 포맷팅된 파라미터 등이 내부적으로 객체 할당을 발생시켜요ㅠ
이로 인해서 GC의 빈도가 늘어나고, 결국 앱의 퍼포먼스 저하로 이어질 수 있답니다!
※ 물론 Timber와 같은 로깅 라이브러리를 활용하면
릴리즈 빌드에서 로그를 비활성화할 수 있지만,
디버그 빌드에서도 의미 없는 로그는 노이즈가 될 수 있기 때문에
정리하는 습관을 들이는 것이 좋습니다~!!
- KEY_MINIMUM_VERSION 값을 "minimum_version" → "min_required_version" 로 변경
2 participants
📌 ISSUE
closed #238
Firebase RemoteConfig를 통한 앱 버전 관리 및 업데이트 전략 적용
📄 Work Description
✨ PR Point
Remote Config의 fetch 주기는 Default로 12h로 설정되어있는 걸 알게 되었습니다.
왜 AppUpdateChecker 라는 네이밍을 했는가?
Repository 라는 건 일반적으로 Domain Object를 영속성 계층에서 조회/저장/수정하는 역할이므로 우선 Repository는 아니고,
Policy Evaluation에 가깝기 때문에 AppUpdateChecker, AppUpdatePolicy를 고민했는데 뭔가 AppUpdateChecker가 더 직관적이여서 선택했습니다.
업데이트 전략
파이어베이스 설정
📸 ScreenShot/Video
Summary by CodeRabbit
New Features
Chores