fix: drift audit batch 3a — Android offline correctness
android / Build + lint + test (push) Has been cancelled
android / Build + lint + test (push) Has been cancelled
Two findings from the 2026-06-02 drift audit (Scribe parent #552): - **#577 (Android)** RequestsViewModel.cancel() called refresh() on BOTH Synced and Queued outcomes. Synced is fine (re-fetch the canonical list); Queued is offline by definition — the optimistic removal at the top of cancel() is already correct, and refresh() on Queued either (a) gets the not-yet-delivered cancelled row back from the server and snaps it into the list (confusing), or (b) fails with a transport error and flips the screen to UiState.Error so the user thinks the cancel failed even though it's queued. Gate refresh() on outcome == Synced; the mutation replayer reconciles when connectivity returns. - **#570 (Android)** LikesRepository.refreshIds() pulled the server's likes list and INSERTed it into cached_likes — but never DELETEd local rows the server no longer surfaces. A cross-device unlike (user likes on web, then unlikes on web) left the entry visible on Android's Liked tab indefinitely with no way to clear short of wiping app data. Add CachedLikeDao.clearForUser + a @Transaction replaceAllForUser that atomically wipes-then-inserts the user's set; refreshIds() uses replaceAllForUser so the local cache is exactly what the server reports. The LOCAL_USER_ID hardcode is its own drift (#576) and stays for now — fixing it needs threading AuthStore.userId through the repo.
This commit is contained in:
+18
@@ -4,6 +4,7 @@ import androidx.room.Dao
|
|||||||
import androidx.room.Insert
|
import androidx.room.Insert
|
||||||
import androidx.room.OnConflictStrategy
|
import androidx.room.OnConflictStrategy
|
||||||
import androidx.room.Query
|
import androidx.room.Query
|
||||||
|
import androidx.room.Transaction
|
||||||
import com.fabledsword.minstrel.cache.db.entities.CachedLikeEntity
|
import com.fabledsword.minstrel.cache.db.entities.CachedLikeEntity
|
||||||
import kotlinx.coroutines.flow.Flow
|
import kotlinx.coroutines.flow.Flow
|
||||||
|
|
||||||
@@ -36,4 +37,21 @@ interface CachedLikeDao {
|
|||||||
"WHERE userId = :userId AND entityType = :entityType AND entityId = :entityId",
|
"WHERE userId = :userId AND entityType = :entityType AND entityId = :entityId",
|
||||||
)
|
)
|
||||||
suspend fun delete(userId: String, entityType: String, entityId: String)
|
suspend fun delete(userId: String, entityType: String, entityId: String)
|
||||||
|
|
||||||
|
@Query("DELETE FROM cached_likes WHERE userId = :userId")
|
||||||
|
suspend fun clearForUser(userId: String)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Atomically replaces the user's entire cached_likes set with
|
||||||
|
* [rows]. Used by [com.fabledsword.minstrel.likes.data.LikesRepository.refreshIds]
|
||||||
|
* so cross-device unlikes (a row that the server no longer
|
||||||
|
* surfaces) get removed from the local cache — drift #570
|
||||||
|
* caught the missing delete pass that left stale Liked tab
|
||||||
|
* entries pointing at tracks the user had unliked elsewhere.
|
||||||
|
*/
|
||||||
|
@Transaction
|
||||||
|
suspend fun replaceAllForUser(userId: String, rows: List<CachedLikeEntity>) {
|
||||||
|
clearForUser(userId)
|
||||||
|
upsertAll(rows)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -111,11 +111,18 @@ class LikesRepository @Inject constructor(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Pulls `GET /api/likes/ids` and reconciles `cached_likes`. Called
|
* Pulls `GET /api/likes/ids` and atomically replaces the user's
|
||||||
* by the LikedTab ViewModel on init (and by the SyncController in
|
* cached_likes set with the server's canonical view. Called by
|
||||||
* Phase 12). Adds rows for IDs the server has but we don't; the
|
* the LikedTab ViewModel on init (and by the SyncController in
|
||||||
* delete-of-stale-rows pass lives in the SyncController where the
|
* Phase 12).
|
||||||
* full reconciliation runs.
|
*
|
||||||
|
* Drift #570 fix: previously this called `upsertAll` only, which
|
||||||
|
* added rows for IDs the server had but never removed rows the
|
||||||
|
* server no longer surfaced — so a cross-device unlike (user
|
||||||
|
* likes on web, then unlikes on web) left the Liked tab on
|
||||||
|
* Android showing the now-unliked entry forever. The atomic
|
||||||
|
* replaceAllForUser DAO method runs a transactional delete-then-
|
||||||
|
* insert so the local set is exactly what the server reports.
|
||||||
*/
|
*/
|
||||||
suspend fun refreshIds() {
|
suspend fun refreshIds() {
|
||||||
val wire = api.ids()
|
val wire = api.ids()
|
||||||
@@ -123,7 +130,7 @@ class LikesRepository @Inject constructor(
|
|||||||
rows += wire.artistIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ARTIST, it) }
|
rows += wire.artistIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ARTIST, it) }
|
||||||
rows += wire.albumIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ALBUM, it) }
|
rows += wire.albumIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ALBUM, it) }
|
||||||
rows += wire.trackIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_TRACK, it) }
|
rows += wire.trackIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_TRACK, it) }
|
||||||
likeDao.upsertAll(rows)
|
likeDao.replaceAllForUser(LOCAL_USER_ID, rows)
|
||||||
}
|
}
|
||||||
|
|
||||||
companion object {
|
companion object {
|
||||||
|
|||||||
+15
-6
@@ -5,6 +5,7 @@ import androidx.lifecycle.viewModelScope
|
|||||||
import com.fabledsword.minstrel.api.ErrorCopy
|
import com.fabledsword.minstrel.api.ErrorCopy
|
||||||
import com.fabledsword.minstrel.events.EventsStream
|
import com.fabledsword.minstrel.events.EventsStream
|
||||||
import com.fabledsword.minstrel.models.RequestRef
|
import com.fabledsword.minstrel.models.RequestRef
|
||||||
|
import com.fabledsword.minstrel.requests.data.CancelOutcome
|
||||||
import com.fabledsword.minstrel.requests.data.RequestsRepository
|
import com.fabledsword.minstrel.requests.data.RequestsRepository
|
||||||
import com.fabledsword.minstrel.shared.UiState
|
import com.fabledsword.minstrel.shared.UiState
|
||||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||||
@@ -71,7 +72,7 @@ class RequestsViewModel @Inject constructor(
|
|||||||
}
|
}
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
try {
|
try {
|
||||||
val (_, updated) = repository.cancel(id)
|
val (outcome, updated) = repository.cancel(id)
|
||||||
if (updated != null) {
|
if (updated != null) {
|
||||||
internal.update { state ->
|
internal.update { state ->
|
||||||
if (state !is UiState.Success) {
|
if (state !is UiState.Success) {
|
||||||
@@ -87,12 +88,20 @@ class RequestsViewModel @Inject constructor(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Refresh fetches the canonical list whether the cancel
|
// Drift #577: only refetch on the Synced outcome. The
|
||||||
// went through live (Synced) or got enqueued (Queued).
|
// Queued path is offline by definition — the optimistic
|
||||||
// The Queued case shows the optimistic removal until the
|
// removal at the top of cancel() is correct, and a
|
||||||
// replayer drains; the next list fetch surfaces the
|
// refresh() call here would either (a) get the
|
||||||
// server's canonical view.
|
// not-yet-delivered cancelled row back from the server
|
||||||
|
// and snap it into the list (confusing), or (b) fail
|
||||||
|
// with a transport error and flip the whole screen to
|
||||||
|
// UiState.Error, making the user think the cancel
|
||||||
|
// failed. The mutation queue will replay the cancel
|
||||||
|
// when connectivity returns; the next on-screen refresh
|
||||||
|
// (pull-to-refresh, navigation back) reconciles.
|
||||||
|
if (outcome == CancelOutcome.Synced) {
|
||||||
refresh()
|
refresh()
|
||||||
|
}
|
||||||
} catch (
|
} catch (
|
||||||
@Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
|
@Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
|
||||||
) {
|
) {
|
||||||
|
|||||||
Reference in New Issue
Block a user