fix(android): drift #576 — LikesRepository uses real userId, clears on switch
android / Build + lint + test (push) Successful in 4m1s
android / Build + lint + test (push) Successful in 4m1s
Final drift audit finding (Scribe parent #552). LikesRepository hardcoded LOCAL_USER_ID = "local" as the cached_likes discriminator since before the auth slice landed. After auth shipped, the app has a real per-user session but every device wrote rows under the same "local" bucket — so sharing an Android device between two Minstrel accounts left the previous user's likes visible to the new user. Changes: - Inject AuthController + ApplicationScope so the repo can read the current user UUID and subscribe to user-switch events. - `currentUserId()` resolves the cached_likes discriminator to `authController.currentUser.value?.id` with the legacy "local" fallback (ANONYMOUS_USER_ID, renamed from LOCAL_USER_ID) so pre-#576 cache rows from existing installs stay queryable until the first authenticated refreshIds() overwrites them. - All eight call sites that used the constant now use the helper: observeLikedArtists/Albums/Tracks, observeIsLiked, likedTrackIds, toggleLike (optimistic upsert + delete), refreshIds (server replace). - init {} subscribes to authController.currentUser; when the signed-in id changes, the OUTGOING user's rows get likeDao.clearForUser. Mostly a hygiene fix — the discriminator already prevents the wrong user from SEEING leaked rows, but without this they pile up forever as different accounts sign in/out on the same device. This closes the final drift audit finding from the 2026-06-02 run. 26 of 26 candidate findings either confirmed-and-shipped (24) or cancelled-as-duplicate (1) or shipped-with-honest-doc-fix (1).
This commit is contained in:
@@ -1,19 +1,23 @@
|
|||||||
package com.fabledsword.minstrel.likes.data
|
package com.fabledsword.minstrel.likes.data
|
||||||
|
|
||||||
import com.fabledsword.minstrel.api.endpoints.LikesApi
|
import com.fabledsword.minstrel.api.endpoints.LikesApi
|
||||||
|
import com.fabledsword.minstrel.auth.AuthController
|
||||||
import com.fabledsword.minstrel.cache.db.dao.CachedAlbumDao
|
import com.fabledsword.minstrel.cache.db.dao.CachedAlbumDao
|
||||||
import com.fabledsword.minstrel.cache.db.dao.CachedArtistDao
|
import com.fabledsword.minstrel.cache.db.dao.CachedArtistDao
|
||||||
import com.fabledsword.minstrel.cache.db.dao.CachedLikeDao
|
import com.fabledsword.minstrel.cache.db.dao.CachedLikeDao
|
||||||
import com.fabledsword.minstrel.cache.db.dao.CachedTrackDao
|
import com.fabledsword.minstrel.cache.db.dao.CachedTrackDao
|
||||||
import com.fabledsword.minstrel.cache.db.entities.CachedLikeEntity
|
import com.fabledsword.minstrel.cache.db.entities.CachedLikeEntity
|
||||||
import com.fabledsword.minstrel.cache.mutations.MutationQueue
|
import com.fabledsword.minstrel.cache.mutations.MutationQueue
|
||||||
|
import com.fabledsword.minstrel.di.ApplicationScope
|
||||||
import com.fabledsword.minstrel.library.data.toDomain
|
import com.fabledsword.minstrel.library.data.toDomain
|
||||||
import com.fabledsword.minstrel.models.AlbumRef
|
import com.fabledsword.minstrel.models.AlbumRef
|
||||||
import com.fabledsword.minstrel.models.ArtistRef
|
import com.fabledsword.minstrel.models.ArtistRef
|
||||||
import com.fabledsword.minstrel.models.TrackRef
|
import com.fabledsword.minstrel.models.TrackRef
|
||||||
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.flow.Flow
|
import kotlinx.coroutines.flow.Flow
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import kotlinx.coroutines.flow.map
|
import kotlinx.coroutines.flow.map
|
||||||
|
import kotlinx.coroutines.launch
|
||||||
import retrofit2.Retrofit
|
import retrofit2.Retrofit
|
||||||
import retrofit2.create
|
import retrofit2.create
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
@@ -24,11 +28,14 @@ import javax.inject.Singleton
|
|||||||
* Flutter `likesControllerProvider` + `cached_likes`-driven Liked tab
|
* Flutter `likesControllerProvider` + `cached_likes`-driven Liked tab
|
||||||
* pattern.
|
* pattern.
|
||||||
*
|
*
|
||||||
* Local `userId` discriminator is a constant for now (`LOCAL_USER_ID`)
|
* Local `userId` discriminator is the server-side user UUID
|
||||||
* because the AuthController hookup lands with Phase 11. Single-user
|
* (resolved via [AuthController.currentUser]); falls back to
|
||||||
* device is the only shape we target; if multi-tenant on one device
|
* [ANONYMOUS_USER_ID] when no user is signed in so existing local
|
||||||
* ever shows up, swap this constant for `AuthStore.userId.value`
|
* cache rows from pre-#576 builds remain queryable until the first
|
||||||
* everywhere and migrate the cached_likes rows.
|
* authenticated [refreshIds] call overwrites them. On user-switch
|
||||||
|
* (sign-out / sign-in as different user) the OUTGOING user's
|
||||||
|
* cached_likes rows are wiped so they don't leak into the new
|
||||||
|
* session — drift #576 audit caught the leak.
|
||||||
*
|
*
|
||||||
* Write path follows `feedback_offline_first_for_server_writes` —
|
* Write path follows `feedback_offline_first_for_server_writes` —
|
||||||
* never fire-and-forget:
|
* never fire-and-forget:
|
||||||
@@ -48,33 +55,64 @@ class LikesRepository @Inject constructor(
|
|||||||
private val artistDao: CachedArtistDao,
|
private val artistDao: CachedArtistDao,
|
||||||
private val trackDao: CachedTrackDao,
|
private val trackDao: CachedTrackDao,
|
||||||
private val mutationQueue: MutationQueue,
|
private val mutationQueue: MutationQueue,
|
||||||
|
private val authController: AuthController,
|
||||||
|
@ApplicationScope private val scope: CoroutineScope,
|
||||||
retrofit: Retrofit,
|
retrofit: Retrofit,
|
||||||
) {
|
) {
|
||||||
private val api: LikesApi = retrofit.create()
|
private val api: LikesApi = retrofit.create()
|
||||||
|
|
||||||
|
init {
|
||||||
|
// Drift #576: clear the outgoing user's cached_likes rows
|
||||||
|
// when the signed-in user changes. Mostly a hygiene fix —
|
||||||
|
// discriminator-based queries already prevent the wrong
|
||||||
|
// user from SEEING leaked rows, but without this the rows
|
||||||
|
// pile up forever as different accounts sign in/out on the
|
||||||
|
// same device.
|
||||||
|
scope.launch {
|
||||||
|
var previousId: String? = null
|
||||||
|
authController.currentUser.collect { user ->
|
||||||
|
val newId = user?.id
|
||||||
|
val prev = previousId
|
||||||
|
if (prev != null && prev != newId) {
|
||||||
|
likeDao.clearForUser(prev)
|
||||||
|
}
|
||||||
|
previousId = newId
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Server-user-uuid for the current session, or [ANONYMOUS_USER_ID]
|
||||||
|
* fallback when there's no signed-in user. Used as the
|
||||||
|
* `cached_likes.userId` discriminator so each account's likes are
|
||||||
|
* stored under its own bucket.
|
||||||
|
*/
|
||||||
|
private fun currentUserId(): String =
|
||||||
|
authController.currentUser.value?.id ?: ANONYMOUS_USER_ID
|
||||||
|
|
||||||
// ── Reads ─────────────────────────────────────────────────────────
|
// ── Reads ─────────────────────────────────────────────────────────
|
||||||
|
|
||||||
fun observeLikedArtists(): Flow<List<ArtistRef>> =
|
fun observeLikedArtists(): Flow<List<ArtistRef>> =
|
||||||
likeDao.observeLikedIdsOfType(LOCAL_USER_ID, ENTITY_ARTIST).map { ids ->
|
likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_ARTIST).map { ids ->
|
||||||
ids.mapNotNull { artistDao.getById(it)?.toDomain() }
|
ids.mapNotNull { artistDao.getById(it)?.toDomain() }
|
||||||
}
|
}
|
||||||
|
|
||||||
fun observeLikedAlbums(): Flow<List<AlbumRef>> =
|
fun observeLikedAlbums(): Flow<List<AlbumRef>> =
|
||||||
likeDao.observeLikedIdsOfType(LOCAL_USER_ID, ENTITY_ALBUM).map { ids ->
|
likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_ALBUM).map { ids ->
|
||||||
ids.mapNotNull { albumDao.getById(it)?.toDomain() }
|
ids.mapNotNull { albumDao.getById(it)?.toDomain() }
|
||||||
}
|
}
|
||||||
|
|
||||||
fun observeLikedTracks(): Flow<List<TrackRef>> =
|
fun observeLikedTracks(): Flow<List<TrackRef>> =
|
||||||
likeDao.observeLikedIdsOfType(LOCAL_USER_ID, ENTITY_TRACK).map { ids ->
|
likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_TRACK).map { ids ->
|
||||||
ids.mapNotNull { trackDao.getById(it)?.toDomain() }
|
ids.mapNotNull { trackDao.getById(it)?.toDomain() }
|
||||||
}
|
}
|
||||||
|
|
||||||
fun observeIsLiked(entityType: String, entityId: String): Flow<Boolean> =
|
fun observeIsLiked(entityType: String, entityId: String): Flow<Boolean> =
|
||||||
likeDao.observeIsLiked(LOCAL_USER_ID, entityType, entityId)
|
likeDao.observeIsLiked(currentUserId(), entityType, entityId)
|
||||||
|
|
||||||
/** One-shot snapshot of the liked track-id set — for the offline pool filter. */
|
/** One-shot snapshot of the liked track-id set — for the offline pool filter. */
|
||||||
suspend fun likedTrackIds(): Set<String> =
|
suspend fun likedTrackIds(): Set<String> =
|
||||||
likeDao.observeLikedIdsOfType(LOCAL_USER_ID, ENTITY_TRACK).first().toSet()
|
likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_TRACK).first().toSet()
|
||||||
|
|
||||||
// ── Writes (optimistic local → best-effort REST → enqueue on fail) ──
|
// ── Writes (optimistic local → best-effort REST → enqueue on fail) ──
|
||||||
|
|
||||||
@@ -85,13 +123,15 @@ class LikesRepository @Inject constructor(
|
|||||||
* if it got enqueued for later replay.
|
* if it got enqueued for later replay.
|
||||||
*/
|
*/
|
||||||
suspend fun toggleLike(entityType: String, entityId: String, desiredState: Boolean): Boolean {
|
suspend fun toggleLike(entityType: String, entityId: String, desiredState: Boolean): Boolean {
|
||||||
// 1. Optimistic Room mutation.
|
// 1. Optimistic Room mutation — scoped to the signed-in user
|
||||||
|
// (or the legacy "local" fallback for pre-#576 rows).
|
||||||
|
val uid = currentUserId()
|
||||||
if (desiredState) {
|
if (desiredState) {
|
||||||
likeDao.upsertAll(
|
likeDao.upsertAll(
|
||||||
listOf(CachedLikeEntity(LOCAL_USER_ID, entityType, entityId)),
|
listOf(CachedLikeEntity(uid, entityType, entityId)),
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
likeDao.delete(LOCAL_USER_ID, entityType, entityId)
|
likeDao.delete(uid, entityType, entityId)
|
||||||
}
|
}
|
||||||
// 2. Best-effort REST call; 3. enqueue on failure.
|
// 2. Best-effort REST call; 3. enqueue on failure.
|
||||||
val kindPath = serverPathFor(entityType)
|
val kindPath = serverPathFor(entityType)
|
||||||
@@ -125,17 +165,24 @@ class LikesRepository @Inject constructor(
|
|||||||
* insert so the local set is exactly what the server reports.
|
* insert so the local set is exactly what the server reports.
|
||||||
*/
|
*/
|
||||||
suspend fun refreshIds() {
|
suspend fun refreshIds() {
|
||||||
|
val uid = currentUserId()
|
||||||
val wire = api.ids()
|
val wire = api.ids()
|
||||||
val rows = mutableListOf<CachedLikeEntity>()
|
val rows = mutableListOf<CachedLikeEntity>()
|
||||||
rows += wire.artistIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ARTIST, it) }
|
rows += wire.artistIds.map { CachedLikeEntity(uid, ENTITY_ARTIST, it) }
|
||||||
rows += wire.albumIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ALBUM, it) }
|
rows += wire.albumIds.map { CachedLikeEntity(uid, ENTITY_ALBUM, it) }
|
||||||
rows += wire.trackIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_TRACK, it) }
|
rows += wire.trackIds.map { CachedLikeEntity(uid, ENTITY_TRACK, it) }
|
||||||
likeDao.replaceAllForUser(LOCAL_USER_ID, rows)
|
likeDao.replaceAllForUser(uid, rows)
|
||||||
}
|
}
|
||||||
|
|
||||||
companion object {
|
companion object {
|
||||||
// TODO(Phase 11): swap for AuthStore.userId.value once auth lands.
|
// Pre-auth fallback discriminator. Drift #576: when no user is
|
||||||
const val LOCAL_USER_ID: String = "local"
|
// signed in OR for cached_likes rows written by pre-#576
|
||||||
|
// builds (which all tagged "local"), reads continue to query
|
||||||
|
// this bucket so the Liked tab isn't suddenly empty after the
|
||||||
|
// upgrade. The first authenticated refreshIds() call writes
|
||||||
|
// rows under the real user UUID; reads then transparently
|
||||||
|
// switch over via currentUserId().
|
||||||
|
const val ANONYMOUS_USER_ID: String = "local"
|
||||||
const val ENTITY_ARTIST: String = "artist"
|
const val ENTITY_ARTIST: String = "artist"
|
||||||
const val ENTITY_ALBUM: String = "album"
|
const val ENTITY_ALBUM: String = "album"
|
||||||
const val ENTITY_TRACK: String = "track"
|
const val ENTITY_TRACK: String = "track"
|
||||||
|
|||||||
Reference in New Issue
Block a user