Skip to content

Conversation

@HI-JIN2
Copy link
Member

@HI-JIN2 HI-JIN2 commented Oct 16, 2025

Summary

firebase remote config 활용 코드를 개선했습니다

Describe your changes

remote config에서는 2가지 정보를 받아옵니다.

1. 앱의 최소 버전 
2. 각 식당 운영 정보

추가로, 인터페이스 - 구현체로 나누는게 좋을 것 같아서 해당 부분 정의 했습니다.

수정 전 수정 후
인터페이스 없이 바로 구현체 인터페이스 - 구현체로 나눔
BaseActivity의 VersionViewmodel에서 RemoteConfigRepository 호출해서 강제업데이트(최소버전) 로직 수행 IntroViewModel에서 호출하도록 변경(굳이 매 화면에서 해야할 이유는 없겠다 싶음)
JSONArray로 식당운영정보 파싱 Gson으로 수정

@HI-JIN2 HI-JIN2 self-assigned this Oct 16, 2025
@HI-JIN2 HI-JIN2 marked this pull request as ready for review October 16, 2025 11:52
@HI-JIN2 HI-JIN2 requested review from PeraSite, Copilot and kangyuri1114 and removed request for PeraSite and kangyuri1114 October 16, 2025 11:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the Firebase Remote Config implementation to follow better architectural patterns. The main goal is to improve code organization by introducing interface-based design and relocating version checking logic to a more appropriate location.

Key changes:

  • Introduced interface-implementation separation for FirebaseRemoteConfigRepository
  • Moved force update logic from BaseActivity to IntroViewModel, eliminating unnecessary checks on every screen
  • Replaced JSONArray parsing with Gson for restaurant information

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
IntroViewModel.kt Added version checking and Remote Config initialization logic with proper error handling
IntroActivity.kt Added UI handling for force update dialog based on version check results
VersionViewModelFactory.kt Removed obsolete factory class
VersionViewModel.kt Removed obsolete ViewModel for version checking
InfoViewModel.kt Simplified to fetch restaurant info on-demand instead of maintaining in-memory cache
InfoBottomSheetFragment.kt Removed unnecessary flow collection and fixed property reference
BaseActivity.kt Removed version checking logic and related dependencies
FirebaseRemoteConfigRepository.kt Created new interface defining repository contract
RestaurantInfo.kt Renamed photoUrl field to image for consistency
DataModule.kt Added Hilt binding for FirebaseRemoteConfigRepository interface
AppModule.kt Removed manual provider in favor of interface binding
FirebaseRemoteConfigRepositoryImpl.kt New implementation with Gson parsing and suspend function support
FirebaseRemoteConfigRepository.kt (data) Deleted old implementation file

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

import com.eatssu.android.domain.model.RestaurantInfo
import com.eatssu.android.domain.repository.FirebaseRemoteConfigRepository
import com.eatssu.common.enums.Restaurant
import com.google.common.reflect.TypeToken
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

Using Guava's TypeToken is not recommended for Android projects. Consider using Gson's TypeToken (com.google.gson.reflect.TypeToken) instead, which is already available in the Gson library and is more appropriate for Android applications.

Suggested change
import com.google.common.reflect.TypeToken
import com.google.gson.reflect.TypeToken

Copilot uses AI. Check for mistakes.
@PeraSite
Copy link
Member

PeraSite commented Nov 6, 2025

FirebaseRemoteConfig에서 fetchAndActivate를 계속 실행하더라도 설정한 setMinimumFetchIntervalInSeconds 값이 지나지 않았으면 Fetch를 하지 않더라구요! Init을 명시적으로 코드 내부에서 해줘야하는 것이 이상해서 제 PR에서는 이 부분을 아예 삭제했는데
https://github.com/EAT-SSU/Android/pull/403/files#diff-6bd33371d969f0fcadbd00845bb27098e7e1fbdcb1980b279411c021199aacfd 를 참조해주세요!

@HI-JIN2
Copy link
Member Author

HI-JIN2 commented Nov 9, 2025

@PeraSite 없앴다는 구절이 여기가 맞을까요??

스크린샷 2025-11-09 오후 3 28 31

@PeraSite
Copy link
Member

PeraSite commented Nov 9, 2025

@PeraSite 없앴다는 구절이 여기가 맞을까요??

스크린샷 2025-11-09 오후 3 28 31

맞습니다! 너무 뭉뚱그려 말씀드린 것 같아 이 부분에 더하자면 제 의견은 다음과 같습니다!

기존 코드는 FirebaseRemoteConfigRepositoryImpl.init()을 초기애 1회 실행시켜줘야 했습니다. 지금은 VersionViewModel.kt가 해당 역할을 수행하고 있죠.
그런데 외부에서 init()을 무조건 호출해줘야 정상 작동하는 코드는 우아하지 않은 것 같아 init() 대신 Kotlin의 init 블록을 사용하면 좋다고 생각해요!

현재는 init 때만 fetchAndActivate를 호출하고 있어, 의도한 바인 '10분에 한번씩 값 업데이트'가 이루어지지 않고 있습니다. 앱 실행 최초 한번에만 Firebase Config를 가져오는 것이죠.
FirebaseRemoteConfig.fetch 함수의 주석을 보면 다음과 같아요. fetchAndActivate를 아무리 많이 호출해도 마지막 fetch로부터 지난 시간이 min fetch interval이 되지 않았으면 로컬에서 가져옵니다.
Starts fetching configs, adhering to the default minimum fetch interval. The fetched configs only take effect after the next activate call. Depending on the time elapsed since the last fetch from the Firebase Remote Config backend, configs are either served from local storage, or fetched from the backend. The default minimum fetch interval can be set with FirebaseRemoteConfigSettings.Builder#setMinimumFetchIntervalInSeconds(long); the static default is 12 hours.
그래서 의도한대로 구현하려면 값을 가져오기전에 무조건 fetchAndActivate를 해야해요!
suspend 내에서 fetchAndActivate().await() 한 후에 값을 가져오면 됩니다.
그렇게 수정하게 되면 기존 init() 함수의 fetchAndActivate 호출은 필요 없어지는거죵!

Copy link
Member

@kangyuri1114 kangyuri1114 left a comment

Choose a reason for hiding this comment

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

좋네요!!! 수고하셨습니다~~

Comment on lines 63 to 74
private suspend fun initializeRemoteConfig() {
try {
firebaseRemoteConfigRepository.init().fold(
onSuccess = {
Timber.d("Firebase Remote Config 초기화 성공")
},
onFailure = { error ->
Timber.e(error, "Firebase Remote Config 초기화 실패")
// Remote Config 초기화 실패해도 앱은 계속 진행
}
Copy link
Member

Choose a reason for hiding this comment

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

전에 runcatching으로Viewmodel 내 api 결과 오류/성공 처리 하자고 했던거같아서요!

요기도 적용하면 어떨까용

private suspend fun initializeRemoteConfig() {
    firebaseRemoteConfigRepository.init()
        .onSuccess { Timber.d("Firebase Remote Config 초기화 성공") }
        .onFailure { error ->
            Timber.e(error, "Firebase Remote Config 초기화 실패")
        }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 함수는 Init을 뷰모델(외부)에서 안하게 되어서 삭제 되었답니다..😮

Copy link
Member

@kangyuri1114 kangyuri1114 left a comment

Choose a reason for hiding this comment

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

fetchAndActivate를 아무리 많이 호출해도 마지막 fetch로부터 지난 시간이 min fetch interval이 되지 않았으면 로컬에서 가져옵니다.

이 말씀은 그럼 기존 코드처럼 매 화면마다 version 체크가 이루어져야 한다는 걸까요?>??

@PeraSite

@HI-JIN2
Copy link
Member Author

HI-JIN2 commented Nov 16, 2025

@PeraSite 님 말씀 참고해서

  1. init 외부에서 -> init 블럭으로 수정
  2. 값을 가져올 때마다 fetchAndActivate() 호출

두 가지 큰 변동이 있습니다! @kangyuri1114 @PeraSite 마지막으로 한 번 더 봐주시면 감사하겠습니다! 663280d

@kangyuri1114
Copy link
Member

확인했습니다! 제가 볼 땐 문제 없어보여요 코드도 더 좋아진 것 같습니다

@HI-JIN2 HI-JIN2 merged commit d997db7 into develop Nov 19, 2025
1 check passed
@HI-JIN2 HI-JIN2 deleted the refactor/firebase branch November 19, 2025 08:17
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.

4 participants