-
Notifications
You must be signed in to change notification settings - Fork 0
Speed up initial loading with upcoming games query #110
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -1,33 +1,26 @@ | ||
| query PagedGames($limit: Int!, $offset: Int!) { | ||
| games(limit: $limit, offset: $offset) { | ||
| id | ||
| city | ||
| date | ||
| gender | ||
| location | ||
| opponentId | ||
| result | ||
| sport | ||
| state | ||
| time | ||
| scoreBreakdown | ||
| utcDate | ||
| team { | ||
| id | ||
| color | ||
| image | ||
| name | ||
| } | ||
| boxScore { | ||
| team | ||
| period | ||
| time | ||
| description | ||
| scorer | ||
| assist | ||
| scoreBy | ||
| corScore | ||
| oppScore | ||
| } | ||
| ...GameListItem | ||
| } | ||
| } | ||
|
|
||
| query InitialGames($startDate: DateTime!, $endDate: DateTime!) { | ||
| gamesByDate(startDate: $startDate, endDate: $endDate) { | ||
| ...GameListItem | ||
| } | ||
| } | ||
|
|
||
| fragment GameListItem on GameType { | ||
| id | ||
| city | ||
| date | ||
| gender | ||
| result | ||
| sport | ||
| time | ||
| team { | ||
| color | ||
| image | ||
| name | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,13 +6,18 @@ import com.cornellappdev.score.util.isValidSport | |
| import com.cornellappdev.score.util.parseColor | ||
| import com.cornellappdev.score.util.parseResultScore | ||
| import com.example.score.GameByIdQuery | ||
| import com.example.score.InitialGamesQuery | ||
| import com.example.score.GamesQuery | ||
| import com.example.score.PagedGamesQuery | ||
| import kotlinx.coroutines.CancellationException | ||
| import kotlinx.coroutines.CoroutineScope | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.flow.asStateFlow | ||
| import kotlinx.coroutines.flow.update | ||
| import kotlinx.coroutines.launch | ||
| import kotlinx.coroutines.withTimeoutOrNull | ||
| import kotlinx.coroutines.sync.Mutex | ||
| import java.time.LocalDate | ||
| import kotlinx.coroutines.withTimeout | ||
| import javax.inject.Inject | ||
| import javax.inject.Singleton | ||
|
|
@@ -24,15 +29,15 @@ private const val PAGE_TIMEOUT_MILLIS = 3000L | |
|
|
||
| /** | ||
| * This is a singleton responsible for fetching and caching all data for Score. | ||
| * Right now, it makes a network request for all possible games. In the future, | ||
| * we should limit this to games only in a certain time range, to prevent the | ||
| * app from slowing down and improve load times. | ||
| * Publishes a small date window first, then loads the full game history. | ||
| */ | ||
| @Singleton | ||
| class ScoreRepository @Inject constructor( | ||
| private val apolloClient: ApolloClient, | ||
| private val appScope: CoroutineScope, | ||
| ) { | ||
| private val gamesFetchMutex = Mutex() | ||
|
|
||
| private val _upcomingGamesFlow = | ||
| MutableStateFlow<ApiResponse<List<Game>>>(ApiResponse.Loading) | ||
| val upcomingGamesFlow = _upcomingGamesFlow.asStateFlow() | ||
|
|
@@ -99,20 +104,44 @@ class ScoreRepository @Inject constructor( | |
| } | ||
|
|
||
| fun fetchGames() = appScope.launch { | ||
| if (!gamesFetchMutex.tryLock()) return@launch | ||
| _upcomingGamesFlow.value = ApiResponse.Loading | ||
|
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. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '90,225p' app/src/main/java/com/cornellappdev/score/model/ScoreRepository.kt
rg -n '_upcomingGamesFlow|fetchGames\(' app/src/main/javaRepository: cuappdev/score-android Length of output: 7836 🏁 Script executed: set -o pipefail
printf '%s\n' '--- ScoreRepository declarations and fetch entry points ---'
sed -n '1,125p' app/src/main/java/com/cornellappdev/score/model/ScoreRepository.kt
printf '%s\n' '--- HomeViewModel refresh/state handling ---'
sed -n '1,110p' app/src/main/java/com/cornellappdev/score/viewmodel/HomeViewModel.kt
printf '%s\n' '--- PastGamesViewModel refresh/state handling ---'
sed -n '1,125p' app/src/main/java/com/cornellappdev/score/viewmodel/PastGamesViewModel.kt
printf '%s\n' '--- ApiResponse and cache-related declarations/usages ---'
rg -n --glob '*.kt' 'sealed class ApiResponse|class ApiResponse|enum class ApiResponse|ApiResponse<|upcomingGames|cache|cached|gameCache|gamesCache' app/src/main/javaRepository: cuappdev/score-android Length of output: 15883 Preserve the success value that existed before the refresh.
🤖 Prompt for AI Agents |
||
| val allGames = mutableListOf<Game>() | ||
| var offset = 0 | ||
| var retries = 0 | ||
| var initialWindow = true | ||
|
|
||
| try { | ||
| while (true) { | ||
| val pageResult = runCatching { | ||
| withTimeout(PAGE_TIMEOUT_MILLIS) { | ||
| apolloClient.query( | ||
| PagedGamesQuery(limit = PAGE_LIMIT, offset = offset) | ||
| ).execute().data?.games | ||
| val pageResult = try { | ||
| withTimeoutOrNull(PAGE_TIMEOUT_MILLIS) { | ||
| if (initialWindow) { | ||
| val today = LocalDate.now() | ||
| apolloClient.query( | ||
| InitialGamesQuery( | ||
| today.atStartOfDay().toString(), | ||
| today.plusDays(30).atStartOfDay().toString() | ||
| ) | ||
| ).execute().toResult().getOrNull()?.gamesByDate | ||
| ?.map { it?.gameListItem } | ||
| } else { | ||
| apolloClient.query( | ||
| PagedGamesQuery(limit = PAGE_LIMIT, offset = offset) | ||
| ).execute().toResult().getOrNull()?.games | ||
| ?.map { it?.gameListItem } | ||
| } | ||
| } | ||
| }.getOrNull() | ||
| } catch (e: CancellationException) { | ||
| throw e | ||
| } catch (e: Exception) { | ||
| null | ||
| } | ||
|
|
||
| // A failed or empty date window falls back to the full fetch. | ||
| if (initialWindow && pageResult.isNullOrEmpty()) { | ||
| initialWindow = false | ||
| continue | ||
| } | ||
|
|
||
| if (pageResult == null) { | ||
| if (retries < MAX_RETRIES) { | ||
|
|
@@ -158,17 +187,32 @@ class ScoreRepository @Inject constructor( | |
|
|
||
| allGames.addAll(pageGames) | ||
|
|
||
| if (initialWindow) { | ||
| if (allGames.isNotEmpty()) { | ||
| _upcomingGamesFlow.value = ApiResponse.Success(allGames.toList()) | ||
| } | ||
| initialWindow = false | ||
| continue | ||
| } | ||
|
|
||
| if (pageResult.size < PAGE_LIMIT) break | ||
| offset += PAGE_LIMIT | ||
| } | ||
|
|
||
| _upcomingGamesFlow.value = | ||
| if (allGames.isNotEmpty()) ApiResponse.Success(allGames) | ||
| if (allGames.isNotEmpty()) ApiResponse.Success(allGames.asReversed().distinctBy { it.id }.asReversed()) | ||
| else if (_upcomingGamesFlow.value is ApiResponse.Success) _upcomingGamesFlow.value | ||
| else ApiResponse.Error | ||
|
|
||
| } catch (e: CancellationException) { | ||
| throw e | ||
| } catch (e: Exception) { | ||
| Log.e("ScoreRepository", "Error fetching upcoming games", e) | ||
| _upcomingGamesFlow.value = ApiResponse.Error | ||
| if (_upcomingGamesFlow.value !is ApiResponse.Success) { | ||
| _upcomingGamesFlow.value = ApiResponse.Error | ||
| } | ||
| } finally { | ||
| gamesFetchMutex.unlock() | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
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
🔎 Supported by static analysis
🏁 Script executed:
Repository: cuappdev/score-android
Length of output: 8631
🏁 Script executed:
Repository: cuappdev/score-android
Length of output: 17080
🏁 Script executed:
Repository: cuappdev/score-android
Length of output: 16985
🏁 Script executed:
Repository: cuappdev/score-android
Length of output: 17292
Do not silently drop a refresh while a fetch is active.
Both refresh view models set
loadedStatetoApiResponse.Loadingbefore callingfetchGames(). IftryLock()fails, no new fetch starts. The active fetch can emit an intermediateSuccess, then assign an equalApiResponse.Successafter pagination.StateFlowsuppresses that equal assignment, so either view model can remain inLoading.Wait for the mutex. Coalescing the call without changing the view-model state handling does not guarantee a new result.
Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents