Compare commits
8
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0cea82984c | ||
|
|
3acac985cd | ||
|
|
d3b40342b4 | ||
|
|
4f99b42844 | ||
|
|
5044e7a055 | ||
|
|
7d45a4e5c7 | ||
|
|
fa0827f668 | ||
|
|
52d53e0044 |
@@ -80,10 +80,16 @@ jobs:
|
||||
|
||||
- name: Upload debug APK
|
||||
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
|
||||
# Gitea Actions runs in GHES-emulation mode; @actions/artifact v2+
|
||||
# (i.e. upload-artifact@v4+) errors with "GHESNotSupportedError".
|
||||
# Pin to @v3 until act_runner or the artifact backend catches up.
|
||||
uses: actions/upload-artifact@v3
|
||||
# Mirrored action, never actions/upload-artifact. @v4+ throws
|
||||
# GHESNotSupportedError client-side on the hostname (no server setting
|
||||
# reaches that check), and @v3 is worse — it reports success while Gitea
|
||||
# serves artifacts back only through the v4 API, so the upload is stored
|
||||
# and invisible to every retrieval path. @v3 is what left 72 unreachable
|
||||
# artifacts on this repo. Pinned by SHA because the mirror auto-syncs;
|
||||
# full URL because DEFAULT_ACTIONS_URL sends bare owner/repo to github.com.
|
||||
# See Scribe issues 2255 / 2270.
|
||||
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
|
||||
with:
|
||||
name: minstrel-android-debug-${{ github.sha }}
|
||||
path: android/app/build/outputs/apk/debug/app-debug.apk
|
||||
if-no-files-found: error
|
||||
|
||||
@@ -131,12 +131,19 @@ jobs:
|
||||
-PMINSTREL_VERSION_CODE=${{ steps.ver.outputs.code }}
|
||||
|
||||
- name: Upload APK as workflow artifact
|
||||
# @v3 because Gitea Actions emulates GHES and the v2 artifact
|
||||
# backend used by upload-artifact@v4 errors with GHESNotSupportedError.
|
||||
uses: actions/upload-artifact@v3
|
||||
# Mirrored action, never actions/upload-artifact — @v4+ refuses on the
|
||||
# hostname, @v3 uploads something Gitea will never serve back. This is
|
||||
# the producing half of a pair: image-release downloads `minstrel-apk`
|
||||
# below with the matching download-artifact mirror. Both must stay on
|
||||
# the v4 protocol — mixing a v3 upload with a v4 download (or the
|
||||
# reverse) yields an empty listing, not an error. See Scribe 2255 / 2270.
|
||||
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
|
||||
with:
|
||||
name: minstrel-apk
|
||||
path: android/app/build/outputs/apk/release/app-release.apk
|
||||
# error, not the default warn: image-release hard-depends on this
|
||||
# artifact existing, so an empty upload must fail here, not there.
|
||||
if-no-files-found: error
|
||||
|
||||
- name: Attach APK to gitea Release
|
||||
shell: bash
|
||||
@@ -238,7 +245,20 @@ jobs:
|
||||
# Tag pushes only — android-release just produced this. Non-tag
|
||||
# builds take the "Bundle latest release APK" path below instead.
|
||||
if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v')
|
||||
uses: actions/download-artifact@v3
|
||||
# Consuming half of the pair — never actions/download-artifact. Same fork,
|
||||
# same reason: upstream's client-side GHES check rejects this hostname
|
||||
# before it connects. bvandeusen/download-artifact mirrors
|
||||
# code.forgejo.org/forgejo/download-artifact.
|
||||
#
|
||||
# SHA below is that fork's `v6` tag. Match on @actions/artifact, NOT on
|
||||
# the action's own version number — the two actions release on unrelated
|
||||
# cadences, and download v5 would pair a ^2.3.2 client with this file's
|
||||
# ^4.0.0 uploader. v6 is the tag whose bundled library major (^4.0.0) is
|
||||
# the same one proven against this instance by the upload side.
|
||||
# Deliberately NOT v7: it moves to node24 and upstream requires runner
|
||||
# >= 2.327.1 for it, which act_runner does not claim to satisfy.
|
||||
# Pinned, not tagged — the mirror auto-syncs every 8h.
|
||||
uses: https://git.fabledsword.com/bvandeusen/download-artifact@8d4e9521a5f7e5f8b6351f341f719f9f45a92a3a
|
||||
with:
|
||||
name: minstrel-apk
|
||||
path: client/
|
||||
|
||||
@@ -210,4 +210,17 @@ dependencies {
|
||||
debugImplementation(libs.compose.ui.test.manifest)
|
||||
}
|
||||
|
||||
tasks.withType<Test> { useJUnitPlatform() }
|
||||
tasks.withType<Test> {
|
||||
useJUnitPlatform()
|
||||
// Print the assertion message + full stack trace for failures. The
|
||||
// default console output gives only "AssertionError at Foo.kt:12", and
|
||||
// for a failure inside a `runTest { }` lambda even that line collapses
|
||||
// to the test function's own line (the assertion frames live in the
|
||||
// suspend-lambda class, which Gradle filters out) — leaving nothing to
|
||||
// debug from when the HTML report isn't reachable, as in CI.
|
||||
testLogging {
|
||||
events("failed")
|
||||
exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
|
||||
showStackTraces = true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import androidx.work.Configuration
|
||||
import coil3.ImageLoader
|
||||
import coil3.SingletonImageLoader
|
||||
import coil3.network.okhttp.OkHttpNetworkFetcherFactory
|
||||
import coil3.request.crossfade
|
||||
import com.fabledsword.minstrel.cache.CacheIndexer
|
||||
import com.fabledsword.minstrel.cache.mutations.MutationReplayer
|
||||
import com.fabledsword.minstrel.cache.sync.SyncController
|
||||
@@ -29,6 +30,10 @@ import okhttp3.OkHttpClient
|
||||
import timber.log.Timber
|
||||
import javax.inject.Inject
|
||||
|
||||
// Cover-art fade-in. Coil skips the transition for memory-cache hits, so
|
||||
// already-loaded art still appears instantly — only a genuine fetch fades.
|
||||
private const val ART_CROSSFADE_MS = 220
|
||||
|
||||
@HiltAndroidApp
|
||||
class MinstrelApplication :
|
||||
Application(),
|
||||
@@ -213,11 +218,18 @@ class MinstrelApplication :
|
||||
* OkHttp client as the network fetcher. The `callFactory` lambda
|
||||
* is invoked lazily so Hilt has time to inject `okHttpClient`
|
||||
* before Coil makes its first request.
|
||||
*
|
||||
* Crossfade is set here rather than per-call so every cover surface
|
||||
* in the app fades its artwork in instead of snapping it. Art
|
||||
* landing a beat after its tile was the most visible pop-in on Home
|
||||
* (issue #2327); `ServerImage` fades its placeholder out over the
|
||||
* same window so the two read as one cross-dissolve.
|
||||
*/
|
||||
override fun newImageLoader(context: android.content.Context): ImageLoader =
|
||||
ImageLoader.Builder(context)
|
||||
.components {
|
||||
add(OkHttpNetworkFetcherFactory(callFactory = { okHttpClient }))
|
||||
}
|
||||
.crossfade(ART_CROSSFADE_MS)
|
||||
.build()
|
||||
}
|
||||
|
||||
+24
-3
@@ -4,6 +4,7 @@ import androidx.room.Dao
|
||||
import androidx.room.Insert
|
||||
import androidx.room.OnConflictStrategy
|
||||
import androidx.room.Query
|
||||
import androidx.room.Transaction
|
||||
import com.fabledsword.minstrel.cache.db.entities.CachedHomeIndexEntity
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
|
||||
@@ -21,12 +22,32 @@ interface CachedHomeIndexDao {
|
||||
)
|
||||
suspend fun getBySection(section: String): List<CachedHomeIndexEntity>
|
||||
|
||||
/** True when Home has any cached section rows to render. */
|
||||
@Query("SELECT EXISTS(SELECT 1 FROM cached_home_index)")
|
||||
suspend fun hasAny(): Boolean
|
||||
|
||||
@Insert(onConflict = OnConflictStrategy.REPLACE)
|
||||
suspend fun upsertAll(rows: List<CachedHomeIndexEntity>)
|
||||
|
||||
/** Replace-all pattern; sync wipes a section then re-inserts. */
|
||||
@Query("DELETE FROM cached_home_index WHERE section = :section")
|
||||
suspend fun deleteBySection(section: String)
|
||||
@Query("DELETE FROM cached_home_index WHERE section IN (:sections)")
|
||||
suspend fun deleteSections(sections: List<String>)
|
||||
|
||||
/**
|
||||
* Swaps every listed section's rows in ONE transaction.
|
||||
*
|
||||
* Atomicity is the point, not just tidiness: Room's
|
||||
* InvalidationTracker only notifies observers after the transaction
|
||||
* commits, so [observeBySection] never sees the empty gap between the
|
||||
* delete and the re-insert. Replacing sections one at a time (and
|
||||
* un-transacted) made each Home row emit `emptyList()` — visibly
|
||||
* collapsing — before refilling, and made the seven sections do it in
|
||||
* sequence rather than as a single content swap.
|
||||
*/
|
||||
@Transaction
|
||||
suspend fun replaceSections(sections: List<String>, rows: List<CachedHomeIndexEntity>) {
|
||||
deleteSections(sections)
|
||||
if (rows.isNotEmpty()) upsertAll(rows)
|
||||
}
|
||||
|
||||
@Query("DELETE FROM cached_home_index")
|
||||
suspend fun clear()
|
||||
|
||||
@@ -14,8 +14,10 @@ import com.fabledsword.minstrel.models.TrackRef
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||
import kotlinx.coroutines.flow.flatMapLatest
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.flow.map
|
||||
import retrofit2.Retrofit
|
||||
import retrofit2.create
|
||||
import javax.inject.Inject
|
||||
@@ -34,9 +36,10 @@ import javax.inject.Singleton
|
||||
* reveals when the fetch lands and Room re-emits. Mirrors Flutter's
|
||||
* per-item tile providers.
|
||||
*
|
||||
* `refreshIndex()` pulls `GET /api/home/index`, replaces each section
|
||||
* in-place (delete-then-insert, so the section Flows re-fire), and
|
||||
* pre-warms the top artists via [HomeArtistPrewarmer].
|
||||
* `refreshIndex()` pulls `GET /api/home/index`, swaps all sections in
|
||||
* one transaction (so the rows update together in a single emission
|
||||
* rather than collapsing and refilling), and pre-warms the top artists
|
||||
* via [HomeArtistPrewarmer].
|
||||
*/
|
||||
@Singleton
|
||||
// Per-section observe accessors (one per Home row) inflate the function
|
||||
@@ -85,72 +88,98 @@ class HomeRepository @Inject constructor(
|
||||
fun observeYouMightLikeArtists(): Flow<List<HomeTile<ArtistRef>>> =
|
||||
observeArtistSection(SECTION_YOU_MIGHT_LIKE_ARTISTS)
|
||||
|
||||
/** True when the index cache already has content on screen to protect. */
|
||||
suspend fun hasCachedIndex(): Boolean = homeIndexDao.hasAny()
|
||||
|
||||
/**
|
||||
* Pulls /api/home/index, replaces each cached_home_index section,
|
||||
* and pre-warms the top artists. The section Flows re-fire on the
|
||||
* index change; missing entity rows hydrate via the on-miss path.
|
||||
* Pulls /api/home/index and swaps every cached_home_index section in
|
||||
* a single transaction, then pre-warms the top artists. Missing
|
||||
* entity rows hydrate via the on-miss path.
|
||||
*
|
||||
* One transaction for all seven sections is deliberate: Room notifies
|
||||
* observers once, on commit, so Home swaps from the old content to
|
||||
* the new in a single emission. Per-section, un-transacted writes
|
||||
* made every row visibly collapse to empty and refill, one after
|
||||
* another (issue #2327).
|
||||
*/
|
||||
suspend fun refreshIndex() {
|
||||
val wire = api.getHomeIndex()
|
||||
replaceSection(SECTION_RECENTLY_ADDED_ALBUMS, "album", wire.recentlyAddedAlbums)
|
||||
replaceSection(SECTION_REDISCOVER_ALBUMS, "album", wire.rediscoverAlbums)
|
||||
replaceSection(SECTION_REDISCOVER_ARTISTS, "artist", wire.rediscoverArtists)
|
||||
replaceSection(SECTION_MOST_PLAYED_TRACKS, "track", wire.mostPlayedTracks)
|
||||
replaceSection(SECTION_LAST_PLAYED_ARTISTS, "artist", wire.lastPlayedArtists)
|
||||
replaceSection(SECTION_YOU_MIGHT_LIKE_ALBUMS, "album", wire.youMightLikeAlbums)
|
||||
replaceSection(SECTION_YOU_MIGHT_LIKE_ARTISTS, "artist", wire.youMightLikeArtists)
|
||||
homeIndexDao.replaceSections(
|
||||
sections = ALL_SECTIONS,
|
||||
rows = rowsFor(SECTION_RECENTLY_ADDED_ALBUMS, "album", wire.recentlyAddedAlbums) +
|
||||
rowsFor(SECTION_REDISCOVER_ALBUMS, "album", wire.rediscoverAlbums) +
|
||||
rowsFor(SECTION_REDISCOVER_ARTISTS, "artist", wire.rediscoverArtists) +
|
||||
rowsFor(SECTION_MOST_PLAYED_TRACKS, "track", wire.mostPlayedTracks) +
|
||||
rowsFor(SECTION_LAST_PLAYED_ARTISTS, "artist", wire.lastPlayedArtists) +
|
||||
rowsFor(SECTION_YOU_MIGHT_LIKE_ALBUMS, "album", wire.youMightLikeAlbums) +
|
||||
rowsFor(SECTION_YOU_MIGHT_LIKE_ARTISTS, "artist", wire.youMightLikeArtists),
|
||||
)
|
||||
prewarmer.warm(
|
||||
wire.rediscoverArtists + wire.lastPlayedArtists + wire.youMightLikeArtists,
|
||||
)
|
||||
}
|
||||
|
||||
private suspend fun replaceSection(section: String, entityType: String, ids: List<String>) {
|
||||
homeIndexDao.deleteBySection(section)
|
||||
if (ids.isEmpty()) return
|
||||
homeIndexDao.upsertAll(
|
||||
ids.mapIndexed { index, id ->
|
||||
CachedHomeIndexEntity(
|
||||
section = section,
|
||||
position = index,
|
||||
entityType = entityType,
|
||||
entityId = id,
|
||||
)
|
||||
},
|
||||
private fun rowsFor(
|
||||
section: String,
|
||||
entityType: String,
|
||||
ids: List<String>,
|
||||
): List<CachedHomeIndexEntity> = ids.mapIndexed { index, id ->
|
||||
CachedHomeIndexEntity(
|
||||
section = section,
|
||||
position = index,
|
||||
entityType = entityType,
|
||||
entityId = id,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The section's ordered entity ids, deduplicated.
|
||||
*
|
||||
* Room re-runs the query on every write to `cached_home_index` — and
|
||||
* `CachedHomeIndexEntity.fetchedAt` is stamped fresh each time — so
|
||||
* comparing whole rows would call every rewrite a change. Comparing
|
||||
* the id list instead means a section whose contents didn't actually
|
||||
* move never restarts the `flatMapLatest` below, which would
|
||||
* otherwise tear down and rebuild all of its tiles' hydration flows
|
||||
* and flicker unchanged tiles (issue #2327).
|
||||
*/
|
||||
private fun observeSectionIds(section: String): Flow<List<String>> =
|
||||
homeIndexDao.observeBySection(section)
|
||||
.map { rows -> rows.map { it.entityId } }
|
||||
.distinctUntilChanged()
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
private fun observeAlbumSection(section: String): Flow<List<HomeTile<AlbumRef>>> =
|
||||
homeIndexDao.observeBySection(section).flatMapLatest { rows ->
|
||||
if (rows.isEmpty()) {
|
||||
observeSectionIds(section).flatMapLatest { ids ->
|
||||
if (ids.isEmpty()) {
|
||||
flowOf(emptyList())
|
||||
} else {
|
||||
combine(rows.map { metadataProvider.observeAlbum(it.entityId) }) { refs ->
|
||||
rows.mapIndexed { i, r -> HomeTile(r.entityId, refs[i]) }
|
||||
combine(ids.map { metadataProvider.observeAlbum(it) }) { refs ->
|
||||
ids.mapIndexed { i, id -> HomeTile(id, refs[i]) }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
private fun observeArtistSection(section: String): Flow<List<HomeTile<ArtistRef>>> =
|
||||
homeIndexDao.observeBySection(section).flatMapLatest { rows ->
|
||||
if (rows.isEmpty()) {
|
||||
observeSectionIds(section).flatMapLatest { ids ->
|
||||
if (ids.isEmpty()) {
|
||||
flowOf(emptyList())
|
||||
} else {
|
||||
combine(rows.map { metadataProvider.observeArtist(it.entityId) }) { refs ->
|
||||
rows.mapIndexed { i, r -> HomeTile(r.entityId, refs[i]) }
|
||||
combine(ids.map { metadataProvider.observeArtist(it) }) { refs ->
|
||||
ids.mapIndexed { i, id -> HomeTile(id, refs[i]) }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
private fun observeTrackSection(section: String): Flow<List<HomeTile<TrackRef>>> =
|
||||
homeIndexDao.observeBySection(section).flatMapLatest { rows ->
|
||||
if (rows.isEmpty()) {
|
||||
observeSectionIds(section).flatMapLatest { ids ->
|
||||
if (ids.isEmpty()) {
|
||||
flowOf(emptyList())
|
||||
} else {
|
||||
combine(rows.map { metadataProvider.observeTrack(it.entityId) }) { refs ->
|
||||
rows.mapIndexed { i, r -> HomeTile(r.entityId, refs[i]) }
|
||||
combine(ids.map { metadataProvider.observeTrack(it) }) { refs ->
|
||||
ids.mapIndexed { i, id -> HomeTile(id, refs[i]) }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -163,5 +192,16 @@ class HomeRepository @Inject constructor(
|
||||
const val SECTION_LAST_PLAYED_ARTISTS = "last_played_artists"
|
||||
const val SECTION_YOU_MIGHT_LIKE_ALBUMS = "you_might_like_albums"
|
||||
const val SECTION_YOU_MIGHT_LIKE_ARTISTS = "you_might_like_artists"
|
||||
|
||||
/** Every section [refreshIndex] owns — the unit of one atomic swap. */
|
||||
val ALL_SECTIONS = listOf(
|
||||
SECTION_RECENTLY_ADDED_ALBUMS,
|
||||
SECTION_REDISCOVER_ALBUMS,
|
||||
SECTION_REDISCOVER_ARTISTS,
|
||||
SECTION_MOST_PLAYED_TRACKS,
|
||||
SECTION_LAST_PLAYED_ARTISTS,
|
||||
SECTION_YOU_MIGHT_LIKE_ALBUMS,
|
||||
SECTION_YOU_MIGHT_LIKE_ARTISTS,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -43,6 +43,7 @@ import androidx.compose.material3.SnackbarHost
|
||||
import androidx.compose.material3.SnackbarHostState
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.remember
|
||||
@@ -85,30 +86,38 @@ import com.fabledsword.minstrel.playlists.widgets.OfflinePoolCard
|
||||
import com.fabledsword.minstrel.playlists.widgets.PlaylistCard
|
||||
import com.fabledsword.minstrel.playlists.widgets.PlaylistPlaceholderCard
|
||||
import com.fabledsword.minstrel.shared.UiState
|
||||
import com.fabledsword.minstrel.shared.UpdateVeilController
|
||||
import com.fabledsword.minstrel.shared.VeilOutcome
|
||||
import com.fabledsword.minstrel.shared.VeilSessionResult
|
||||
import com.fabledsword.minstrel.shared.VeilSettleState
|
||||
import com.fabledsword.minstrel.shared.asCacheFirstStateFlow
|
||||
import com.fabledsword.minstrel.shared.widgets.ArtSettleTracker
|
||||
import com.fabledsword.minstrel.shared.widgets.EmptyState
|
||||
import com.fabledsword.minstrel.shared.widgets.ErrorRetry
|
||||
import com.fabledsword.minstrel.shared.widgets.HorizontalScrollRow
|
||||
import com.fabledsword.minstrel.shared.widgets.LocalArtSettleTracker
|
||||
import com.fabledsword.minstrel.shared.widgets.MinstrelTopAppBar
|
||||
import com.fabledsword.minstrel.shared.widgets.PullToRefreshScaffold
|
||||
import com.fabledsword.minstrel.shared.widgets.SkeletonAlbumTile
|
||||
import com.fabledsword.minstrel.shared.widgets.SkeletonArtistTile
|
||||
import com.fabledsword.minstrel.shared.widgets.SkeletonSectionHeader
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.async
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.coroutineScope
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.flow.receiveAsFlow
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.flow.filter
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import javax.inject.Inject
|
||||
|
||||
private const val SHARE_STOP_TIMEOUT_MS = 5_000L
|
||||
@@ -120,16 +129,22 @@ private const val BOTTOM_PADDING_FOR_MINIPLAYER_DP = 140
|
||||
private const val RECENTLY_ADDED_GRID_ROWS = 2
|
||||
private const val RECENTLY_ADDED_GRID_HEIGHT_DP = 440
|
||||
|
||||
// "Updating your mixes…" veil (automatic refresh). Held through the pull
|
||||
// plus VEIL_SETTLE_MS so per-tile hydration lands behind it before it wipes
|
||||
// off; near-opaque (VEIL_ALPHA) so the section churn never bleeds through.
|
||||
private const val VEIL_SETTLE_MS = 500L
|
||||
// "Updating your mixes…" veil. UpdateVeilController decides both whether it
|
||||
// appears at all — only when a refresh actually changes something — and how
|
||||
// long it stays, by watching the screen settle rather than by a fixed delay,
|
||||
// which lowered it while tiles and artwork were still landing (#2327).
|
||||
// Near-opaque (VEIL_ALPHA) so the section churn never bleeds through.
|
||||
private const val VEIL_WIPE_MS = 280
|
||||
private const val VEIL_ALPHA = 0.96f
|
||||
private const val VEIL_SPINNER_DP = 22
|
||||
private const val VEIL_SPINNER_STROKE_DP = 2
|
||||
private const val VEIL_LABEL_GAP_DP = 12
|
||||
|
||||
// Backstop on how long a manual pull keeps its own indicator while waiting
|
||||
// for its successor — the veil, or the "already up to date" snackbar — so the
|
||||
// two never both vanish for a frame mid-handoff.
|
||||
private const val PULL_HANDOFF_TIMEOUT_MS = 2_000L
|
||||
|
||||
// ─── State ───────────────────────────────────────────────────────────
|
||||
|
||||
data class HomeSections(
|
||||
@@ -184,10 +199,13 @@ class HomeViewModel @Inject constructor(
|
||||
initialValue = false,
|
||||
)
|
||||
|
||||
private val poolMessages = Channel<String>(Channel.BUFFERED)
|
||||
private val snackbarMessages = Channel<String>(Channel.BUFFERED)
|
||||
|
||||
/** Transient snackbar messages from offline-pool taps. */
|
||||
val transientMessages: Flow<String> = poolMessages.receiveAsFlow()
|
||||
/**
|
||||
* Transient snackbar messages: offline-pool taps, playback failures, and
|
||||
* the outcome of a refresh the user explicitly asked for.
|
||||
*/
|
||||
val transientMessages: Flow<String> = snackbarMessages.receiveAsFlow()
|
||||
|
||||
/**
|
||||
* Copy for the most recent /home/index refresh failure; null once a
|
||||
@@ -197,38 +215,13 @@ class HomeViewModel @Inject constructor(
|
||||
*/
|
||||
private val refreshError = MutableStateFlow<String?>(null)
|
||||
|
||||
private val updatingInternal = MutableStateFlow(false)
|
||||
|
||||
/**
|
||||
* True while an automatic background refresh (the 03:00 daily rebuild
|
||||
* or a reconnect re-pull) is repopulating Home. Drives the "Updating
|
||||
* your mixes…" veil so the section churn — delete-then-insert in
|
||||
* [HomeRepository.refreshIndex] plus per-tile hydration — happens
|
||||
* hidden behind the veil instead of on screen. Manual pull-to-refresh
|
||||
* and cold start are NOT veiled (they own the pull spinner / skeleton).
|
||||
* Cover-art loads in flight on Home, reported by every [ServerImage]
|
||||
* under [LocalArtSettleTracker]. The veil waits on this so artwork
|
||||
* arriving a beat after its tile lands behind the veil rather than
|
||||
* popping in on screen.
|
||||
*/
|
||||
val isUpdating: StateFlow<Boolean> = updatingInternal.asStateFlow()
|
||||
|
||||
init {
|
||||
refresh()
|
||||
// Screen-level auto-recovery (issue #1245): a Home that failed to
|
||||
// load while the server was unreachable re-pulls itself the moment
|
||||
// health returns — same idiom as SyncController, one layer up.
|
||||
// Veiled: content is already on screen and would otherwise churn.
|
||||
viewModelScope.launch {
|
||||
networkStatus.recoveries().collect { refreshBehindVeil() }
|
||||
}
|
||||
// #968: the daily 03:00 rebuild (and manual refresh) emit
|
||||
// playlist.system_rebuilt; re-pull Home so the system-playlist tiles
|
||||
// and You-might-like rows reflect the new snapshot without a manual
|
||||
// reload. Mirrors the web SSE consumer. Veiled so the multi-section
|
||||
// rebuild churn hides behind "Updating your mixes…".
|
||||
viewModelScope.launch {
|
||||
eventsStream.events
|
||||
.filter { it.kind == "playlist.system_rebuilt" }
|
||||
.collect { refreshBehindVeil() }
|
||||
}
|
||||
}
|
||||
val artTracker = ArtSettleTracker()
|
||||
|
||||
/**
|
||||
* Tap an offline pool: shuffle + play its cached tracks. Empty
|
||||
@@ -241,7 +234,7 @@ class HomeViewModel @Inject constructor(
|
||||
OfflinePoolKind.LIKED -> shuffleSource.liked()
|
||||
}.shuffled()
|
||||
if (tracks.isEmpty()) {
|
||||
poolMessages.trySend("No cached ${kind.label} tracks yet")
|
||||
snackbarMessages.trySend("No cached ${kind.label} tracks yet")
|
||||
} else {
|
||||
player.setQueue(tracks, initialIndex = 0, source = "offline:${kind.name}")
|
||||
}
|
||||
@@ -278,14 +271,14 @@ class HomeViewModel @Inject constructor(
|
||||
try {
|
||||
val detail = libraryRepository.refreshAlbumDetail(albumId)
|
||||
if (detail.tracks.isEmpty()) {
|
||||
poolMessages.trySend("This album has no tracks to play.")
|
||||
snackbarMessages.trySend("This album has no tracks to play.")
|
||||
} else {
|
||||
player.setQueue(detail.tracks, initialIndex = 0, source = "album:$albumId")
|
||||
}
|
||||
} catch (
|
||||
@Suppress("TooGenericExceptionCaught") e: Throwable,
|
||||
) {
|
||||
poolMessages.trySend(
|
||||
snackbarMessages.trySend(
|
||||
"Couldn't start playback: ${ErrorCopy.fromThrowable(e)}",
|
||||
)
|
||||
}
|
||||
@@ -303,14 +296,14 @@ class HomeViewModel @Inject constructor(
|
||||
try {
|
||||
val tracks = libraryRepository.fetchArtistTracks(artistId).shuffled()
|
||||
if (tracks.isEmpty()) {
|
||||
poolMessages.trySend("This artist has no tracks to play.")
|
||||
snackbarMessages.trySend("This artist has no tracks to play.")
|
||||
} else {
|
||||
player.setQueue(tracks, initialIndex = 0, source = "artist:$artistId")
|
||||
}
|
||||
} catch (
|
||||
@Suppress("TooGenericExceptionCaught") e: Throwable,
|
||||
) {
|
||||
poolMessages.trySend(
|
||||
snackbarMessages.trySend(
|
||||
"Couldn't start playback: ${ErrorCopy.fromThrowable(e)}",
|
||||
)
|
||||
}
|
||||
@@ -328,52 +321,66 @@ class HomeViewModel @Inject constructor(
|
||||
suspend fun playPlaylist(playlist: PlaylistRef) {
|
||||
viewModelScope.launch {
|
||||
playPlaylistShuffled(playlist, playlistsRepository, player) {
|
||||
poolMessages.trySend(it)
|
||||
snackbarMessages.trySend(it)
|
||||
}
|
||||
}.join()
|
||||
}
|
||||
|
||||
/**
|
||||
* Pulls both /home/index and the playlists list. Returns the Job
|
||||
* for the combined refresh so a pull-to-refresh wrapper can await
|
||||
* actual completion before hiding the indicator.
|
||||
* Pulls /home/index, the playlists list and the system-playlist
|
||||
* status. Returns true when the load-bearing /home/index pull
|
||||
* succeeded — the veil controller retries on false and reports the
|
||||
* outcome, so this must report failure rather than swallow it.
|
||||
*/
|
||||
fun refresh(): Job = viewModelScope.launch {
|
||||
refreshError.value = null
|
||||
val home = launch {
|
||||
// /home/index is the load-bearing pull: its failure drives the
|
||||
// empty-cache Error state. A failure over a populated cache
|
||||
// stays silent — cached sections beat a full-screen error.
|
||||
private suspend fun runRefresh(): Boolean = coroutineScope {
|
||||
// /home/index is the load-bearing pull: its failure drives the
|
||||
// empty-cache Error state. A failure over a populated cache
|
||||
// stays silent — cached sections beat a full-screen error.
|
||||
//
|
||||
// Cleared on success, NOT at the start of each attempt: with the
|
||||
// veil's retries, clearing up front made a failing cold start
|
||||
// flash the "Welcome to Minstrel" empty state (empty cache + no
|
||||
// error reads as Empty) between one attempt and the next.
|
||||
val home = async {
|
||||
runCatching { homeRepository.refreshIndex() }
|
||||
.onSuccess { refreshError.value = null }
|
||||
.onFailure { refreshError.value = ErrorCopy.fromThrowable(it) }
|
||||
.isSuccess
|
||||
}
|
||||
val lists = launch { runCatching { playlistsRepository.refreshList() } }
|
||||
val status = launch {
|
||||
runCatching { homeRepository.getSystemPlaylistsStatus() }
|
||||
.onSuccess { systemStatusInternal.value = it }
|
||||
}
|
||||
home.join()
|
||||
lists.join()
|
||||
status.join()
|
||||
home.await()
|
||||
}
|
||||
|
||||
/**
|
||||
* Automatic background refresh with the "Updating your mixes…" veil
|
||||
* raised (see [isUpdating]). Used by the daily-rebuild + reconnect
|
||||
* paths where Home is already on screen. Holds the veil through the
|
||||
* pull plus a short settle so per-tile hydration lands behind it, then
|
||||
* lets it wipe off. Overlapping automatic refreshes are rare enough
|
||||
* (once-daily rebuild, reconnect) that a plain flag beats a counter.
|
||||
* The Error state's explicit Retry button. User-initiated, so it gets
|
||||
* the controller's retries and reports its outcome; over an empty cache
|
||||
* there's no content to protect, so no veil goes up.
|
||||
*/
|
||||
private fun refreshBehindVeil() {
|
||||
viewModelScope.launch {
|
||||
updatingInternal.value = true
|
||||
try {
|
||||
refresh().join()
|
||||
delay(VEIL_SETTLE_MS)
|
||||
} finally {
|
||||
updatingInternal.value = false
|
||||
}
|
||||
fun retry() = veil.request(userInitiated = true)
|
||||
|
||||
/**
|
||||
* Manual pull-to-refresh. Goes behind the veil like every other refresh
|
||||
* (operator call, 2026-07-31: the churn a pull causes is identical to
|
||||
* the automatic paths, and a small spinner didn't hide it).
|
||||
*
|
||||
* Suspends until the veil has taken over OR the session has finished,
|
||||
* so the pull indicator hands off to exactly one successor: the veil if
|
||||
* content changed, the "Already up to date" snackbar if it didn't. The
|
||||
* timeout is only a backstop against a session that outlives it.
|
||||
*/
|
||||
suspend fun refreshFromPull() {
|
||||
val before = veil.finishedSessions.value
|
||||
veil.request(userInitiated = true)
|
||||
withTimeoutOrNull(PULL_HANDOFF_TIMEOUT_MS) {
|
||||
combine(veil.visible, veil.finishedSessions) { veiled, finished ->
|
||||
veiled || finished != before
|
||||
}.first { it }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -425,6 +432,124 @@ class HomeViewModel @Inject constructor(
|
||||
else -> UiState.Empty
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Updating veil ───────────────────────────────────────────────
|
||||
// Declared after uiState: these initialisers read it, and Kotlin runs
|
||||
// property initialisers and init blocks in declaration order.
|
||||
|
||||
/**
|
||||
* What the veil watches to decide Home has stopped moving: the whole
|
||||
* rendered state, plus how many covers are still loading.
|
||||
*
|
||||
* [UiState.Success] wraps a [HomeSections] data class, so any visible
|
||||
* change — a section swapping ids, one tile hydrating from skeleton to
|
||||
* album — changes this value and re-arms the veil's quiet window.
|
||||
*
|
||||
* Unhydrated tiles deliberately do NOT gate `quiescent`. A tile whose
|
||||
* on-miss fetch soft-fails keeps a null value indefinitely
|
||||
* ([MetadataProvider] swallows those errors), so treating "no
|
||||
* skeletons left" as the settle condition would pin the veil to its
|
||||
* hard ceiling on every refresh. They're covered by the content key
|
||||
* instead: each tile that lands re-arms the window, and once they stop
|
||||
* landing the screen is genuinely still.
|
||||
*/
|
||||
private val settleSignal: Flow<VeilSettleState> =
|
||||
combine(uiState, artTracker.inFlight) { state, artInFlight ->
|
||||
VeilSettleState(
|
||||
contentKey = state,
|
||||
hasContent = state is UiState.Success,
|
||||
quiescent = artInFlight == 0,
|
||||
)
|
||||
}
|
||||
|
||||
private val veil = UpdateVeilController(
|
||||
scope = viewModelScope,
|
||||
settleSignal = settleSignal,
|
||||
shouldVeil = {
|
||||
// Only worth hiding churn when there's already content to
|
||||
// hide. A cold load over an empty cache keeps its skeleton —
|
||||
// veiling that would replace a useful affordance with an
|
||||
// opaque panel. `hasCachedIndex` is the honest check: uiState
|
||||
// still reads Loading until the screen subscribes, so on a
|
||||
// process restore over a warm cache it would say "no content"
|
||||
// right before the cache emits.
|
||||
uiState.value is UiState.Success || homeRepository.hasCachedIndex()
|
||||
},
|
||||
onSessionEnd = ::reportRefreshOutcome,
|
||||
work = ::runRefresh,
|
||||
)
|
||||
|
||||
/**
|
||||
* Tells the user how a refresh *they asked for* went, in the one case
|
||||
* the veil can't: when nothing changed there's no veil to see, and a
|
||||
* pull that produces no visible response at all reads as broken.
|
||||
*
|
||||
* Only user-initiated sessions say anything. The same outcome from a
|
||||
* background check — the initial load, the 03:00 rebuild, a reconnect —
|
||||
* is noise, and "Already up to date" on every launch would be worse
|
||||
* than silence (operator's call, 2026-07-31).
|
||||
*/
|
||||
private fun reportRefreshOutcome(result: VeilSessionResult) {
|
||||
if (!result.userInitiated) return
|
||||
when (result.outcome) {
|
||||
// The veil was the feedback.
|
||||
VeilOutcome.CHANGED -> return
|
||||
VeilOutcome.UNCHANGED -> snackbarMessages.trySend("Already up to date")
|
||||
VeilOutcome.FAILED -> snackbarMessages.trySend("Couldn't check for updates")
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True while the "Updating your mixes…" veil should be raised.
|
||||
*
|
||||
* Raised only when a refresh actually changes what's on screen, and then
|
||||
* held until Home settles — tiles hydrated, artwork loaded — instead of
|
||||
* for a fixed delay after the network pull returns (issue #2327). A
|
||||
* refresh that returns what's already cached shows no veil at all;
|
||||
* [reportRefreshOutcome] tells the user instead, if they asked.
|
||||
*/
|
||||
val isUpdating: StateFlow<Boolean> = veil.visible
|
||||
|
||||
init {
|
||||
// Every refresh path goes through the controller, which decides
|
||||
// per session whether to raise the veil. That includes the initial
|
||||
// load: over a warm cache it's a full re-pull that churns every
|
||||
// section, and it used to run completely unveiled.
|
||||
veil.request()
|
||||
// Screen-level auto-recovery (issue #1245): a Home that failed to
|
||||
// load while the server was unreachable re-pulls itself the moment
|
||||
// health returns — same idiom as SyncController, one layer up.
|
||||
// This is also the recovery that keeps trying after the veil has
|
||||
// given up and lowered; the controller sets no latch against it.
|
||||
viewModelScope.launch {
|
||||
networkStatus.recoveries().collect { veil.request() }
|
||||
}
|
||||
// Server-side changes that rewrite what Home renders (#968 and
|
||||
// the 2026-07-31 widening) re-pull behind the veil. Mirrors the
|
||||
// web SSE consumer.
|
||||
viewModelScope.launch {
|
||||
eventsStream.events
|
||||
.filter { it.kind in VEILED_EVENT_KINDS }
|
||||
.collect { veil.request() }
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/**
|
||||
* Events that change what Home shows. `playlist.system_rebuilt`
|
||||
* is the 03:00 daily rebuild; the other `playlist.*` kinds move
|
||||
* the Playlists and Songs-like rows; `scan.run_finished` changes
|
||||
* Recently added (and Home never reacted to it at all before).
|
||||
*/
|
||||
private val VEILED_EVENT_KINDS = setOf(
|
||||
"playlist.system_rebuilt",
|
||||
"playlist.created",
|
||||
"playlist.updated",
|
||||
"playlist.deleted",
|
||||
"playlist.tracks_changed",
|
||||
"scan.run_finished",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Screen ──────────────────────────────────────────────────────────
|
||||
@@ -454,14 +579,20 @@ fun HomeScreen(
|
||||
val offline by viewModel.offline.collectAsStateWithLifecycle()
|
||||
val updating by viewModel.isUpdating.collectAsStateWithLifecycle()
|
||||
PullToRefreshScaffold(
|
||||
onRefresh = { viewModel.refresh().join() },
|
||||
onRefresh = { viewModel.refreshFromPull() },
|
||||
modifier = Modifier.fillMaxSize().padding(inner),
|
||||
) {
|
||||
Box(Modifier.fillMaxSize()) {
|
||||
HomeStateCrossfade(state, systemStatus, offline, navController, viewModel)
|
||||
// Automatic-refresh veil: the daily rebuild / reconnect
|
||||
// churn hides behind an "Updating your mixes…" wipe. Manual
|
||||
// pull owns the PullToRefreshBox spinner instead.
|
||||
// Every cover below reports its load state to the tracker,
|
||||
// so the veil can wait for artwork instead of guessing.
|
||||
CompositionLocalProvider(
|
||||
LocalArtSettleTracker provides viewModel.artTracker,
|
||||
) {
|
||||
HomeStateCrossfade(state, systemStatus, offline, navController, viewModel)
|
||||
}
|
||||
// Refresh veil: rebuild / reconnect / pull / event churn all
|
||||
// hide behind an "Updating your mixes…" wipe that stays up
|
||||
// until the screen has actually stopped moving.
|
||||
UpdatingVeil(visible = updating)
|
||||
}
|
||||
}
|
||||
@@ -499,7 +630,7 @@ private fun HomeStateCrossfade(
|
||||
is UiState.Error -> ErrorRetry(
|
||||
title = "Couldn't load home",
|
||||
message = s.message,
|
||||
onRetry = { viewModel.refresh() },
|
||||
onRetry = { viewModel.retry() },
|
||||
)
|
||||
is UiState.Success -> HomeSuccessContent(
|
||||
sections = s.data,
|
||||
|
||||
@@ -0,0 +1,315 @@
|
||||
package com.fabledsword.minstrel.shared
|
||||
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.FlowPreview
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.debounce
|
||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import java.util.concurrent.atomic.AtomicBoolean
|
||||
|
||||
// Once raised, the veil stays up at least this long. Without a floor a
|
||||
// no-op refresh wipes on and straight back off, which reads as a glitch.
|
||||
private const val DEFAULT_MIN_HOLD_MS = 900L
|
||||
|
||||
// The screen must stop changing for this long before the veil lowers.
|
||||
// Every content change re-arms it, so a refresh that lands in stages
|
||||
// (index → tile hydration → artwork) holds the veil across all of them.
|
||||
private const val DEFAULT_QUIET_MS = 700L
|
||||
|
||||
// Hard ceiling on visibility. A refresh that never settles must not
|
||||
// strand the user behind an opaque veil; the work itself is NOT capped.
|
||||
private const val DEFAULT_MAX_HOLD_MS = 12_000L
|
||||
|
||||
// Attempts per session. Retrying behind the veil is the point: a pull
|
||||
// that fails on the first try gets another go before the user sees
|
||||
// anything, instead of the veil wiping off over unchanged content.
|
||||
private const val DEFAULT_ATTEMPTS = 3
|
||||
private const val DEFAULT_RETRY_BACKOFF_MS = 600L
|
||||
|
||||
/** Tunables for [UpdateVeilController]; defaults are the Home values. */
|
||||
data class VeilTimings(
|
||||
val minHoldMs: Long = DEFAULT_MIN_HOLD_MS,
|
||||
val quietMs: Long = DEFAULT_QUIET_MS,
|
||||
val maxHoldMs: Long = DEFAULT_MAX_HOLD_MS,
|
||||
val attempts: Int = DEFAULT_ATTEMPTS,
|
||||
val retryBackoffMs: Long = DEFAULT_RETRY_BACKOFF_MS,
|
||||
)
|
||||
|
||||
/**
|
||||
* A snapshot of everything that visibly moves on the veiled screen.
|
||||
*
|
||||
* @param contentKey any value whose equality tracks what's rendered — a
|
||||
* change means the screen moved, and re-arms the quiet timer.
|
||||
* @param hasContent true when real content (not a skeleton or an empty
|
||||
* state) is on screen. The veil waits for this before raising: there's
|
||||
* nothing to hide until there's something to hide.
|
||||
* @param quiescent false while something is still landing (artwork
|
||||
* loading, tiles hydrating). The veil will not lower until this is
|
||||
* true, up to [VeilTimings.maxHoldMs].
|
||||
*/
|
||||
data class VeilSettleState(
|
||||
val contentKey: Any?,
|
||||
val hasContent: Boolean,
|
||||
val quiescent: Boolean,
|
||||
)
|
||||
|
||||
/** What a finished session did, so callers can report it if they want. */
|
||||
enum class VeilOutcome {
|
||||
/** Content changed, and the veil covered the churn. */
|
||||
CHANGED,
|
||||
|
||||
/** The refresh worked, but nothing on screen moved — already current. */
|
||||
UNCHANGED,
|
||||
|
||||
/** Every attempt failed. */
|
||||
FAILED,
|
||||
}
|
||||
|
||||
/**
|
||||
* One session's result, plus whether a user explicitly asked for it.
|
||||
*
|
||||
* [userInitiated] is what lets a caller tell feedback from noise: a user
|
||||
* who pulled to refresh is owed an answer even when the answer is "nothing
|
||||
* changed", while the same outcome from a background check is noise.
|
||||
*/
|
||||
data class VeilSessionResult(
|
||||
val outcome: VeilOutcome,
|
||||
val userInitiated: Boolean,
|
||||
)
|
||||
|
||||
/**
|
||||
* Drives an "updating" overlay from *observed content change and settling*
|
||||
* rather than from a fixed delay.
|
||||
*
|
||||
* The problem this replaces: a veil held for `refresh().join() + 500ms`
|
||||
* lowers while the screen is still moving, because finishing the network
|
||||
* pull is nowhere near the end of the visible work — the pull writes id
|
||||
* lists, then tiles hydrate one by one, then artwork loads. And a plain
|
||||
* `isUpdating` Boolean set in a `finally` gets cleared by whichever of
|
||||
* two overlapping refreshes finishes first, wiping the veil off mid-update
|
||||
* (issue #2327).
|
||||
*
|
||||
* So instead: run [work], raise only if the content actually changes, then
|
||||
* hold until [settleSignal] reports the screen has stopped changing for
|
||||
* [VeilTimings.quietMs] AND is quiescent — bounded below by
|
||||
* [VeilTimings.minHoldMs] so it can never flash, and above by
|
||||
* [VeilTimings.maxHoldMs] so it can never strand.
|
||||
*
|
||||
* The raise is deliberately *reactive*: a refresh that returns what's
|
||||
* already on screen — the common case on a launch over a warm cache —
|
||||
* raises nothing at all, because a veil over an unchanged screen hides
|
||||
* nothing and only delays first paint. The cost is that the veil arrives
|
||||
* one emission after the change, so a single atomic content swap shows
|
||||
* through; everything messier that follows it (tile hydration, then
|
||||
* artwork) still lands behind the veil.
|
||||
*
|
||||
* Overlapping triggers extend the running session instead of racing it,
|
||||
* so the veil stays up continuously rather than lowering and re-raising.
|
||||
*
|
||||
* Failure is quiet at this layer: [work] gets [VeilTimings.attempts] tries
|
||||
* behind the veil, and if they all fail the veil simply wipes off over the
|
||||
* cached content. Giving up ends only *this* session — it sets no latch and
|
||||
* blocks nothing, so the caller's own recovery paths (reconnect re-pull,
|
||||
* freshness sweeps, the next event, a manual pull) keep retrying afterwards
|
||||
* exactly as before. Callers that want to surface a failure can do it from
|
||||
* [onSessionEnd] instead.
|
||||
*
|
||||
* @param work one refresh attempt; returns true when it succeeded.
|
||||
* @param shouldVeil sampled at session start — "is there cached content
|
||||
* this refresh is about to overwrite?". False means a cold load, where
|
||||
* a skeleton is the right affordance, and the work runs unveiled.
|
||||
* @param onSessionEnd called once per finished session, on the controller's
|
||||
* coroutine. Use it for user-facing feedback the veil itself can't give.
|
||||
*/
|
||||
class UpdateVeilController(
|
||||
private val scope: CoroutineScope,
|
||||
private val settleSignal: Flow<VeilSettleState>,
|
||||
private val shouldVeil: suspend () -> Boolean,
|
||||
private val timings: VeilTimings = VeilTimings(),
|
||||
private val onSessionEnd: (VeilSessionResult) -> Unit = {},
|
||||
private val work: suspend () -> Boolean,
|
||||
) {
|
||||
private val visibleInternal = MutableStateFlow(false)
|
||||
|
||||
/** True while the veil should be drawn over the screen. */
|
||||
val visible: StateFlow<Boolean> = visibleInternal.asStateFlow()
|
||||
|
||||
private val finishedInternal = MutableStateFlow(0)
|
||||
|
||||
/**
|
||||
* Increments as each session ends. Lets a caller wait for "this
|
||||
* refresh is done" without knowing whether a veil ever went up —
|
||||
* a pull-to-refresh indicator needs exactly that, since an unchanged
|
||||
* refresh never raises one.
|
||||
*/
|
||||
val finishedSessions: StateFlow<Int> = finishedInternal.asStateFlow()
|
||||
|
||||
// Conflated: a burst of triggers (reconnect + rebuild event arriving
|
||||
// together) collapses into one follow-up pass, not a queue of them.
|
||||
private val requests = Channel<Unit>(Channel.CONFLATED)
|
||||
|
||||
// Sticky across a conflated burst: conflation drops the older token, so
|
||||
// the "a user asked for this" bit can't ride on it. If ANY coalesced
|
||||
// trigger was the user's, the session still owes them an answer.
|
||||
private val userAsked = AtomicBoolean(false)
|
||||
|
||||
init {
|
||||
// One consumer, so sessions are serialised by construction: two
|
||||
// triggers can never each own a piece of the veil's state.
|
||||
scope.launch {
|
||||
while (true) {
|
||||
requests.receive()
|
||||
runSession()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ask for a refresh. Safe to call from any trigger at any rate —
|
||||
* calls arriving during a session extend it rather than starting a
|
||||
* competing one.
|
||||
*
|
||||
* @param userInitiated true when a person explicitly asked (pull to
|
||||
* refresh, a Retry button), which is what [VeilSessionResult] carries
|
||||
* through to [onSessionEnd].
|
||||
*/
|
||||
fun request(userInitiated: Boolean = false) {
|
||||
if (userInitiated) userAsked.set(true)
|
||||
requests.trySend(Unit)
|
||||
}
|
||||
|
||||
private suspend fun runSession() {
|
||||
// Sampled at both ends of the work: a trigger folded in mid-session
|
||||
// (see [drainWork]) may have been the user's, and they're still owed
|
||||
// an answer for it.
|
||||
val askedAtStart = userAsked.getAndSet(false)
|
||||
if (!shouldVeil()) {
|
||||
// Cold load: the skeleton is the right affordance, so no veil.
|
||||
// Succeeding here did change the screen — from nothing to
|
||||
// something — so it reports CHANGED, never "already up to date".
|
||||
val ok = drainWork()
|
||||
finish(succeeded = ok, changed = ok, userInitiated = askedAtStart)
|
||||
return
|
||||
}
|
||||
val raised = CompletableDeferred<Unit>()
|
||||
val raiser = scope.launch { raiseWhenContentChanges(raised) }
|
||||
// Floor and ceiling are measured from the raise, not the request,
|
||||
// so a late raise still gets its full no-flash minimum.
|
||||
val floor = scope.launch {
|
||||
raised.await()
|
||||
delay(timings.minHoldMs)
|
||||
}
|
||||
val ceiling = scope.launch {
|
||||
raised.await()
|
||||
delay(timings.maxHoldMs)
|
||||
visibleInternal.value = false
|
||||
}
|
||||
var succeeded = false
|
||||
try {
|
||||
succeeded = drainWork()
|
||||
// Always wait for the settle, never conditionally on `visible`:
|
||||
// work that finishes without suspending would otherwise reach
|
||||
// here before the raiser has been dispatched, tear the session
|
||||
// down, and leave the churn uncovered. This wait is also what
|
||||
// makes `raised.isCompleted` below a trustworthy "did anything
|
||||
// change?" — a change landing just after the pull returns still
|
||||
// gets seen.
|
||||
withTimeoutOrNull(timings.maxHoldMs) { awaitSettled() }
|
||||
// Honour the no-flash minimum before lowering. Deliberately in
|
||||
// the try and not the finally: on cancellation the scope is
|
||||
// going away and nothing will render the veil, so the floor is
|
||||
// pointless there — and a finally that suspends is a finally
|
||||
// that can resist teardown.
|
||||
if (raised.isCompleted) floor.join()
|
||||
} finally {
|
||||
raiser.cancel()
|
||||
ceiling.cancel()
|
||||
floor.cancel()
|
||||
visibleInternal.value = false
|
||||
finish(
|
||||
succeeded = succeeded,
|
||||
changed = raised.isCompleted,
|
||||
userInitiated = askedAtStart,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Raises the veil the moment the screen's content differs from what was
|
||||
* already on it — and never, if this refresh turns out to be a no-op.
|
||||
*
|
||||
* The baseline is the first state that HAS content, not simply the first
|
||||
* state: over a warm cache the cached rows paint a moment after the
|
||||
* session starts, and treating that first paint as "a change" would veil
|
||||
* every launch, which is the whole thing this avoids.
|
||||
*/
|
||||
private suspend fun raiseWhenContentChanges(raised: CompletableDeferred<Unit>) {
|
||||
val baseline = settleSignal.first { it.hasContent }
|
||||
settleSignal.first { it.hasContent && it.contentKey != baseline.contentKey }
|
||||
visibleInternal.value = true
|
||||
raised.complete(Unit)
|
||||
}
|
||||
|
||||
private fun finish(succeeded: Boolean, changed: Boolean, userInitiated: Boolean) {
|
||||
val outcome = when {
|
||||
!succeeded -> VeilOutcome.FAILED
|
||||
changed -> VeilOutcome.CHANGED
|
||||
else -> VeilOutcome.UNCHANGED
|
||||
}
|
||||
onSessionEnd(
|
||||
VeilSessionResult(
|
||||
outcome = outcome,
|
||||
// Fold in a mid-session request from the user.
|
||||
userInitiated = userInitiated || userAsked.getAndSet(false),
|
||||
),
|
||||
)
|
||||
finishedInternal.update { it + 1 }
|
||||
}
|
||||
|
||||
/** True when the refresh eventually succeeded. */
|
||||
private suspend fun drainWork(): Boolean {
|
||||
var succeeded: Boolean
|
||||
do {
|
||||
succeeded = runWorkWithRetries()
|
||||
// A trigger that arrived mid-session gets folded into this one.
|
||||
} while (requests.tryReceive().isSuccess)
|
||||
return succeeded
|
||||
}
|
||||
|
||||
private suspend fun runWorkWithRetries(): Boolean {
|
||||
repeat(timings.attempts) { attempt ->
|
||||
if (work()) return true
|
||||
if (attempt < timings.attempts - 1) {
|
||||
delay(timings.retryBackoffMs * (attempt + 1))
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* Suspends until the screen has been unchanged for
|
||||
* [VeilTimings.quietMs] and reports itself quiescent.
|
||||
*
|
||||
* `debounce` is what makes this hold across a staged update: every
|
||||
* change restarts the window, so the veil lowers only once emissions
|
||||
* actually stop. `first { quiescent }` then rejects a quiet-but-
|
||||
* still-loading moment and waits for the next lull.
|
||||
*/
|
||||
@OptIn(FlowPreview::class)
|
||||
private suspend fun awaitSettled() {
|
||||
settleSignal
|
||||
.distinctUntilChanged()
|
||||
.debounce(timings.quietMs)
|
||||
.first { it.quiescent }
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
package com.fabledsword.minstrel.shared.widgets
|
||||
|
||||
import androidx.compose.runtime.Stable
|
||||
import androidx.compose.runtime.staticCompositionLocalOf
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.update
|
||||
|
||||
/**
|
||||
* Counts the cover-art loads that are currently in flight, so a
|
||||
* screen-level overlay can wait for the artwork to actually land instead
|
||||
* of guessing with a fixed delay.
|
||||
*
|
||||
* Artwork is the most visible pop-in on Home: a tile can be fully
|
||||
* hydrated (title, artist, counts all present) and still snap its cover
|
||||
* in a second later, which is exactly the churn the "Updating your
|
||||
* mixes…" veil exists to hide. The refresh coroutine can't see that —
|
||||
* it finishes long before Coil does — so the composition reports it
|
||||
* upward here instead.
|
||||
*
|
||||
* [ServerImage] reports into whatever tracker it finds in
|
||||
* [LocalArtSettleTracker], which means every art surface in the app
|
||||
* participates for free. Only *composed* images are counted, so a
|
||||
* LazyRow's off-screen tiles are correctly ignored — the count tracks
|
||||
* the pop-in a user can actually see.
|
||||
*
|
||||
* Provide one per screen that needs it (typically owned by the
|
||||
* screen's ViewModel so its refresh logic can read [inFlight]):
|
||||
*
|
||||
* CompositionLocalProvider(LocalArtSettleTracker provides vm.artTracker) { ... }
|
||||
*/
|
||||
@Stable
|
||||
class ArtSettleTracker {
|
||||
private val inFlightInternal = MutableStateFlow(0)
|
||||
|
||||
/**
|
||||
* How many on-screen images are still loading. Zero means the
|
||||
* artwork has settled — every composed cover has either drawn or
|
||||
* failed to a fallback.
|
||||
*/
|
||||
val inFlight: StateFlow<Int> = inFlightInternal.asStateFlow()
|
||||
|
||||
fun begin() {
|
||||
inFlightInternal.update { it + 1 }
|
||||
}
|
||||
|
||||
fun end() {
|
||||
// Floor at zero: a decrement can outlive its increment when a
|
||||
// tile is disposed mid-load and the count must not go negative
|
||||
// and wedge "settled" off forever.
|
||||
inFlightInternal.update { (it - 1).coerceAtLeast(0) }
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The tracker [ServerImage] reports load state to, or null on screens
|
||||
* that don't care (the default) — reporting is then a no-op.
|
||||
*/
|
||||
val LocalArtSettleTracker = staticCompositionLocalOf<ArtSettleTracker?> { null }
|
||||
@@ -1,25 +1,40 @@
|
||||
package com.fabledsword.minstrel.shared.widgets
|
||||
|
||||
import androidx.compose.animation.core.animateFloatAsState
|
||||
import androidx.compose.animation.core.tween
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.fillMaxSize
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.DisposableEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.draw.alpha
|
||||
import androidx.compose.ui.layout.ContentScale
|
||||
import coil3.compose.AsyncImage
|
||||
import coil3.compose.AsyncImagePainter
|
||||
import com.fabledsword.minstrel.shared.resolveServerUrl
|
||||
|
||||
// The fallback fades out as the artwork crossfades in (Coil's crossfade is
|
||||
// configured globally on the ImageLoader in MinstrelApplication). Matching
|
||||
// durations makes the swap read as one cross-dissolve; without the fade the
|
||||
// placeholder icon vanished a frame before the cover appeared, which is the
|
||||
// "art popping in" the Home veil exists to hide (issue #2327).
|
||||
private const val FALLBACK_FADE_MS = 220
|
||||
|
||||
/**
|
||||
* Renders a server-hosted image, resolving relative URLs centrally so
|
||||
* every cover surface loads consistently. Shows [fallback] when the URL
|
||||
* is blank/unresolvable, while the image is still loading, and when the
|
||||
* load fails — so a tile is never left blank (e.g. art not yet backfilled,
|
||||
* which the "You might like" row hits often).
|
||||
*
|
||||
* In-flight loads are reported to [LocalArtSettleTracker] when a screen
|
||||
* provides one, so a screen-level overlay can wait for artwork to land
|
||||
* instead of guessing with a fixed delay.
|
||||
*/
|
||||
@Composable
|
||||
fun ServerImage(
|
||||
@@ -40,6 +55,23 @@ fun ServerImage(
|
||||
var state by remember(resolved) {
|
||||
mutableStateOf<AsyncImagePainter.State>(AsyncImagePainter.State.Empty)
|
||||
}
|
||||
// Empty counts as loading: it's the pre-request state, so treating it
|
||||
// as settled would let a screen overlay lower before Coil even starts.
|
||||
val loading = state is AsyncImagePainter.State.Empty ||
|
||||
state is AsyncImagePainter.State.Loading
|
||||
val tracker = LocalArtSettleTracker.current
|
||||
DisposableEffect(tracker, loading) {
|
||||
if (loading) tracker?.begin()
|
||||
// Balanced by construction: the effect re-runs when `loading` flips
|
||||
// (decrement, then no re-increment) and disposes when a tile leaves
|
||||
// the composition mid-load (scrolled away).
|
||||
onDispose { if (loading) tracker?.end() }
|
||||
}
|
||||
val fallbackAlpha by animateFloatAsState(
|
||||
targetValue = if (loading || state is AsyncImagePainter.State.Error) 1f else 0f,
|
||||
animationSpec = tween(FALLBACK_FADE_MS),
|
||||
label = "art-fallback",
|
||||
)
|
||||
Box(modifier = modifier, contentAlignment = Alignment.Center) {
|
||||
AsyncImage(
|
||||
model = resolved,
|
||||
@@ -48,10 +80,10 @@ fun ServerImage(
|
||||
contentScale = contentScale,
|
||||
onState = { state = it },
|
||||
)
|
||||
if (state is AsyncImagePainter.State.Loading ||
|
||||
state is AsyncImagePainter.State.Error
|
||||
) {
|
||||
fallback()
|
||||
if (fallbackAlpha > 0f) {
|
||||
Box(Modifier.alpha(fallbackAlpha), contentAlignment = Alignment.Center) {
|
||||
fallback()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+314
@@ -0,0 +1,314 @@
|
||||
package com.fabledsword.minstrel.shared
|
||||
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.TestScope
|
||||
import kotlinx.coroutines.test.advanceTimeBy
|
||||
import kotlinx.coroutines.test.runCurrent
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.jupiter.api.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
private const val WORK_MS = 1_000L
|
||||
private const val CHURN_ROUNDS = 5
|
||||
private const val ART_IN_FLIGHT = 3
|
||||
private const val SUCCEED_ON_ATTEMPT = 3
|
||||
private const val QUIET_WINDOWS_TO_OUTLAST = 3
|
||||
|
||||
// Time to let a session finish once the screen has stopped changing: the
|
||||
// retry backoffs, the quiet window and the minimum hold all fit inside it,
|
||||
// while staying well under maxHoldMs. That gap matters — if a drain ran
|
||||
// past the ceiling, "the veil lowered" would no longer distinguish
|
||||
// "it settled" from "it gave up", which is the whole point of these tests.
|
||||
private const val DRAIN_MS = 3_000L
|
||||
|
||||
/**
|
||||
* The veil's job is to go up only when content actually changes, and then to
|
||||
* stay up until the screen has stopped moving. Each test pins one of the ways
|
||||
* the original fixed-delay implementation got that wrong (issue #2327).
|
||||
*
|
||||
* The controller is built on `backgroundScope` throughout: its consumer
|
||||
* loop runs forever, so hanging it off the test's own scope would stop
|
||||
* `runTest` from ever completing.
|
||||
*
|
||||
* Consequence, and the reason every wait below is an explicit
|
||||
* `advanceTimeBy`: **`advanceUntilIdle()` is useless here.** It advances
|
||||
* only while *foreground* work remains, and everything this controller
|
||||
* does lives in `backgroundScope` — so it returns having run nothing, and
|
||||
* assertions land on a session that never started (CI run 3163 failed all
|
||||
* seven of these with "expected 3, actual 0" and friends). Drive the clock
|
||||
* deliberately instead; don't "simplify" these back to advanceUntilIdle.
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class UpdateVeilControllerTest {
|
||||
|
||||
private val timings = VeilTimings(
|
||||
minHoldMs = 900,
|
||||
quietMs = 700,
|
||||
maxHoldMs = 12_000,
|
||||
attempts = 3,
|
||||
retryBackoffMs = 600,
|
||||
)
|
||||
|
||||
/** Drives the settle signal by hand: content key, presence, art count. */
|
||||
private class FakeScreen(hasContent: Boolean = true) {
|
||||
val state = MutableStateFlow(Triple(0, hasContent, 0))
|
||||
|
||||
val signal = state.map { (key, hasContent, art) ->
|
||||
VeilSettleState(contentKey = key, hasContent = hasContent, quiescent = art == 0)
|
||||
}
|
||||
|
||||
/** Content visibly changed — what the veil exists to cover. */
|
||||
fun churn() {
|
||||
state.value = state.value.copy(first = state.value.first + 1)
|
||||
}
|
||||
|
||||
fun artLoading(count: Int) {
|
||||
state.value = state.value.copy(third = count)
|
||||
}
|
||||
|
||||
fun contentAppears() {
|
||||
state.value = state.value.copy(second = true)
|
||||
}
|
||||
}
|
||||
|
||||
private fun TestScope.controllerOn(
|
||||
screen: FakeScreen,
|
||||
shouldVeil: suspend () -> Boolean = { true },
|
||||
onSessionEnd: (VeilSessionResult) -> Unit = {},
|
||||
work: suspend () -> Boolean,
|
||||
) = UpdateVeilController(
|
||||
scope = backgroundScope,
|
||||
settleSignal = screen.signal,
|
||||
shouldVeil = shouldVeil,
|
||||
timings = timings,
|
||||
onSessionEnd = onSessionEnd,
|
||||
work = work,
|
||||
)
|
||||
|
||||
/** Records every visibility transition, so an extra raise can't hide. */
|
||||
private fun TestScope.recordVisibility(controller: UpdateVeilController): List<Boolean> {
|
||||
val seen = mutableListOf<Boolean>()
|
||||
backgroundScope.launch { controller.visible.collect { seen.add(it) } }
|
||||
return seen
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `veil outlasts content that keeps churning after the pull returns`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
val controller = controllerOn(screen) { true }
|
||||
|
||||
controller.request()
|
||||
runCurrent()
|
||||
// The pull's write lands: content changed, so the veil goes up.
|
||||
screen.churn()
|
||||
runCurrent()
|
||||
assertTrue(controller.visible.value, "veil is up while the screen is still moving")
|
||||
|
||||
// Tiles hydrating one after another, each inside the quiet window.
|
||||
// The old implementation had already wiped off after a flat 500ms.
|
||||
repeat(CHURN_ROUNDS) {
|
||||
advanceTimeBy(timings.quietMs / 2)
|
||||
screen.churn()
|
||||
runCurrent()
|
||||
assertTrue(controller.visible.value, "veil must hold across staged churn")
|
||||
}
|
||||
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
assertFalse(controller.visible.value, "veil lowers once the screen goes quiet")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `veil waits for artwork to finish loading`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
val controller = controllerOn(screen) { true }
|
||||
|
||||
screen.artLoading(ART_IN_FLIGHT)
|
||||
controller.request()
|
||||
runCurrent()
|
||||
screen.churn()
|
||||
runCurrent()
|
||||
// Well past the quiet window and the floor — but art is still in
|
||||
// flight, so lowering now would show the covers popping in.
|
||||
advanceTimeBy(timings.minHoldMs + timings.quietMs * QUIET_WINDOWS_TO_OUTLAST)
|
||||
assertTrue(controller.visible.value, "veil must wait on in-flight art")
|
||||
|
||||
screen.artLoading(0)
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
assertFalse(controller.visible.value, "veil lowers once art has landed")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `retries quietly, then veils the churn the successful attempt produces`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
var attempts = 0
|
||||
val controller = controllerOn(screen) {
|
||||
attempts++
|
||||
val succeeded = attempts >= SUCCEED_ON_ATTEMPT // fail twice
|
||||
// Only a pull that worked writes anything.
|
||||
if (succeeded) screen.churn()
|
||||
succeeded
|
||||
}
|
||||
val seen = recordVisibility(controller)
|
||||
|
||||
controller.request()
|
||||
runCurrent()
|
||||
// A failed pull changes nothing, so there is nothing to hide yet —
|
||||
// the retries happen with no veil at all.
|
||||
assertFalse(controller.visible.value, "no veil over a pull that changed nothing")
|
||||
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
assertEquals(SUCCEED_ON_ATTEMPT, attempts, "retries until the pull succeeds")
|
||||
assertEquals(
|
||||
listOf(false, true, false),
|
||||
seen,
|
||||
"the veil went up once, over the churn the retry finally produced",
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `giving up is silent, reports FAILED, and does not block later requests`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
val results = mutableListOf<VeilSessionResult>()
|
||||
var attempts = 0
|
||||
var succeed = false
|
||||
val controller = controllerOn(screen, onSessionEnd = { results += it }) {
|
||||
attempts++
|
||||
succeed
|
||||
}
|
||||
|
||||
controller.request(userInitiated = true)
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
assertEquals(timings.attempts, attempts, "exhausts its attempts")
|
||||
assertFalse(controller.visible.value, "no veil — a failed pull changed nothing")
|
||||
assertEquals(VeilOutcome.FAILED, results.single().outcome)
|
||||
assertTrue(results.single().userInitiated, "the user asked, so they're owed an answer")
|
||||
|
||||
// Giving up must not latch anything off — the reconnect-driven
|
||||
// recovery still gets to try again later.
|
||||
succeed = true
|
||||
controller.request()
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
assertEquals(timings.attempts + 1, attempts, "a later request still runs")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `overlapping requests extend one veil instead of racing it`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
var started = 0
|
||||
val controller = controllerOn(screen) {
|
||||
started++
|
||||
delay(WORK_MS)
|
||||
screen.churn()
|
||||
true
|
||||
}
|
||||
val seen = recordVisibility(controller)
|
||||
|
||||
// Reconnect and the rebuild event arriving together is what made the
|
||||
// old Boolean flag clear mid-update: whichever pull finished first
|
||||
// wiped the veil off while the other was still running.
|
||||
controller.request()
|
||||
runCurrent()
|
||||
controller.request()
|
||||
advanceTimeBy(WORK_MS + 1)
|
||||
assertTrue(controller.visible.value, "second trigger extends the same veil")
|
||||
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
assertEquals(2, started, "the mid-session trigger still did its pull")
|
||||
assertEquals(listOf(false, true, false), seen, "one veil session, not two")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a never-settling screen still releases the veil at the ceiling`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
val controller = controllerOn(screen) {
|
||||
screen.churn()
|
||||
true
|
||||
}
|
||||
|
||||
screen.artLoading(1) // an image that never completes
|
||||
controller.request()
|
||||
advanceTimeBy(timings.maxHoldMs + 1)
|
||||
assertFalse(controller.visible.value, "the hard ceiling must never strand the user")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unchanged refresh never raises the veil and reports UNCHANGED`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
val results = mutableListOf<VeilSessionResult>()
|
||||
// Succeeds without writing anything — the common case on a launch
|
||||
// over a warm cache, where the server returns what's already cached.
|
||||
val controller = controllerOn(screen, onSessionEnd = { results += it }) { true }
|
||||
val seen = recordVisibility(controller)
|
||||
|
||||
controller.request(userInitiated = true)
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
|
||||
assertEquals(listOf(false), seen, "a veil over an unchanged screen would hide nothing")
|
||||
assertEquals(VeilOutcome.UNCHANGED, results.single().outcome)
|
||||
assertTrue(results.single().userInitiated, "so the caller can say 'already up to date'")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `cached content painting is not mistaken for a change`() = runTest {
|
||||
// Warm cache that hasn't painted yet: the rows arrive a moment after
|
||||
// the session starts. Treating that first paint as churn would veil
|
||||
// every single launch.
|
||||
val screen = FakeScreen(hasContent = false)
|
||||
val controller = controllerOn(screen) { true }
|
||||
|
||||
controller.request()
|
||||
runCurrent()
|
||||
screen.contentAppears()
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
|
||||
assertFalse(controller.visible.value, "first paint is not churn")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a background trigger coalescing with the user's does not swallow their answer`() =
|
||||
runTest {
|
||||
val screen = FakeScreen()
|
||||
val results = mutableListOf<VeilSessionResult>()
|
||||
val controller = controllerOn(screen, onSessionEnd = { results += it }) { true }
|
||||
|
||||
// Conflation drops the older token, so the "a user asked" bit
|
||||
// cannot ride on it — it's tracked separately for exactly this.
|
||||
controller.request(userInitiated = true)
|
||||
controller.request()
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
|
||||
assertTrue(
|
||||
results.first().userInitiated,
|
||||
"the user's request must not be conflated away",
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cold load runs unveiled and is never reported as already up to date`() = runTest {
|
||||
val screen = FakeScreen()
|
||||
val results = mutableListOf<VeilSessionResult>()
|
||||
var ran = false
|
||||
val controller = controllerOn(
|
||||
screen,
|
||||
shouldVeil = { false }, // empty cache: the skeleton owns this
|
||||
onSessionEnd = { results += it },
|
||||
) {
|
||||
ran = true
|
||||
true
|
||||
}
|
||||
|
||||
controller.request(userInitiated = true)
|
||||
advanceTimeBy(DRAIN_MS)
|
||||
|
||||
assertTrue(ran, "the refresh still happens")
|
||||
assertFalse(controller.visible.value, "but no veil over a skeleton")
|
||||
// It went from nothing to something — that IS a change.
|
||||
assertEquals(VeilOutcome.CHANGED, results.single().outcome)
|
||||
}
|
||||
}
|
||||
@@ -44,4 +44,64 @@ None.
|
||||
- **Go toolchain pin.** `go.mod` is on `go 1.25.0` because `golang.org/x/crypto v0.51.0` declares 1.25 as its minimum. `ci-go:1.26` satisfies this with headroom. Future `x/crypto` bumps that move the Go floor should be paired with an image-tag bump in this file + the workflows.
|
||||
- **In-app update channel polling.** `release.yml` polls Gitea's release-asset API for up to 15 min on tag pushes to fetch the APK that `flutter.yml` is concurrently attaching to the same release. The asset eventually appears because `flutter.yml` and `release.yml` run in parallel on the same tag; if the polling times out, the server image ships without the bundled update channel (graceful degradation, not a build failure).
|
||||
- **Cache server reachability.** `test-web.yml` does NOT use `cache: 'npm'` on `actions/setup-node` — the Gitea Actions cache server isn't reachable from this runner's container network and `setup-node` was burning ~4m41s on ETIMEDOUT before failing open. With the migration to `ci-go:1.26`, `setup-node` is removed entirely (Node is in the image). The cache concern reappears if a future change re-introduces a network-dependent action.
|
||||
- **Artifacts — use the mirrored actions, never `actions/{upload,download}-artifact`.**
|
||||
```yaml
|
||||
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
|
||||
uses: https://git.fabledsword.com/bvandeusen/download-artifact@8d4e9521a5f7e5f8b6351f341f719f9f45a92a3a
|
||||
```
|
||||
Upstream's `@v4+` cannot work against this instance and no server-side change
|
||||
will help: `isGhes()` rejects any hostname that isn't `github.com` /
|
||||
`*.ghe.com` / `*.localhost` and throws before it opens a connection, so the
|
||||
server is never asked what it supports. `@v3` is worse — it reports success,
|
||||
and Gitea then serves artifacts back only through the v4 API
|
||||
(`content_encoding = application/zip`), so a v3 upload is stored but invisible
|
||||
to every retrieval path. A green job producing nothing retrievable; that is how
|
||||
72 unreachable artifacts accumulated on this repo. Scribe issues 2255 / 2270.
|
||||
|
||||
Both are pull mirrors of the Forgejo project's forks
|
||||
(`code.forgejo.org/forgejo/{upload,download}-artifact`, one commit on upstream
|
||||
disabling that check), mirrored so CI depends on commits we hold and pinned by
|
||||
SHA because the mirrors auto-sync every 8h — a moved upstream tag would
|
||||
otherwise silently change what runs.
|
||||
|
||||
**Match the pins on `@actions/artifact`, not on the actions' own version
|
||||
numbers.** The two actions release on unrelated cadences, so equal version
|
||||
numbers do NOT mean a compatible pair — upload `v5` bundles `@actions/artifact`
|
||||
^4.0.0 while download `v5` bundles ^2.3.2. The pins above are upload **v5** and
|
||||
download **v6**, which is the pairing that puts ^4.0.0 on both sides. This
|
||||
matters because `release.yml` is a producer/consumer pair — `android-release`
|
||||
uploads `minstrel-apk`, `image-release` downloads it — and a protocol mismatch
|
||||
across it yields an empty listing rather than an error, exactly the silent
|
||||
failure this entry exists to prevent.
|
||||
|
||||
| tag | `@actions/artifact` | runtime |
|
||||
|---|---|---|
|
||||
| upload v4 | ^2.1.1 | node20 |
|
||||
| **upload v5** ← pinned | **^4.0.0** | node20 |
|
||||
| download v4 | ^2.1.1 | node20 |
|
||||
| download v5 | ^2.3.2 | node20 |
|
||||
| **download v6** ← pinned | **^4.0.0** | node20 |
|
||||
| download v7 | ^5.0.0 | **node24** |
|
||||
|
||||
The only true protocol break in this history was **v3 → v4** (upstream:
|
||||
"Downloading artifacts that were created from `actions/upload-artifact@v3` and
|
||||
below are not supported"); v4-and-up are one family. Later majors are mostly
|
||||
ergonomics and runtime — upload v4 forbids re-uploading a name and caps a job
|
||||
at 500 artifacts; download v5 made by-ID extraction match by-name.
|
||||
|
||||
**Do not jump the download pin to v7.** That major is a runner requirement, not
|
||||
a feature change: it moves to `runs.using: node24` and upstream states it
|
||||
"requires a minimum Actions Runner version of 2.327.1 … if you are using
|
||||
self-hosted runners, ensure they are updated before upgrading." act_runner is
|
||||
not GitHub's runner and makes no such version claim, so node24 is unverified
|
||||
here. Everything currently pinned is node20.
|
||||
|
||||
Upload steps set `if-no-files-found: error` rather than the default `warn`, so
|
||||
an upload that matches nothing fails its own job instead of failing the
|
||||
consumer later.
|
||||
|
||||
Retrieval: `GET /api/v1/repos/{owner}/{repo}/actions/runs/{run_id}/artifacts`
|
||||
for the id (global run id, not the repo-scoped run number), then
|
||||
`…/actions/artifacts/{id}/zip`. The workstation has no `unzip` — use
|
||||
`python3 -m zipfile -e`.
|
||||
- **Friction asks.** None pending. The two images cover everything Minstrel needs.
|
||||
|
||||
Reference in New Issue
Block a user