From dbcadf0f93559562bfb0d8f8617ae630eee91e47 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 18:35:54 -0400 Subject: [PATCH] =?UTF-8?q?fix(android):=20drift=20#576=20=E2=80=94=20Like?= =?UTF-8?q?sRepository=20uses=20real=20userId,=20clears=20on=20switch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../minstrel/likes/data/LikesRepository.kt | 85 ++++++++++++++----- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt index 0151a506..c876c7b6 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt @@ -1,19 +1,23 @@ package com.fabledsword.minstrel.likes.data 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.CachedArtistDao import com.fabledsword.minstrel.cache.db.dao.CachedLikeDao import com.fabledsword.minstrel.cache.db.dao.CachedTrackDao import com.fabledsword.minstrel.cache.db.entities.CachedLikeEntity import com.fabledsword.minstrel.cache.mutations.MutationQueue +import com.fabledsword.minstrel.di.ApplicationScope import com.fabledsword.minstrel.library.data.toDomain import com.fabledsword.minstrel.models.AlbumRef import com.fabledsword.minstrel.models.ArtistRef import com.fabledsword.minstrel.models.TrackRef +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch import retrofit2.Retrofit import retrofit2.create import javax.inject.Inject @@ -24,11 +28,14 @@ import javax.inject.Singleton * Flutter `likesControllerProvider` + `cached_likes`-driven Liked tab * pattern. * - * Local `userId` discriminator is a constant for now (`LOCAL_USER_ID`) - * because the AuthController hookup lands with Phase 11. Single-user - * device is the only shape we target; if multi-tenant on one device - * ever shows up, swap this constant for `AuthStore.userId.value` - * everywhere and migrate the cached_likes rows. + * Local `userId` discriminator is the server-side user UUID + * (resolved via [AuthController.currentUser]); falls back to + * [ANONYMOUS_USER_ID] when no user is signed in so existing local + * cache rows from pre-#576 builds remain queryable until the first + * 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` — * never fire-and-forget: @@ -48,33 +55,64 @@ class LikesRepository @Inject constructor( private val artistDao: CachedArtistDao, private val trackDao: CachedTrackDao, private val mutationQueue: MutationQueue, + private val authController: AuthController, + @ApplicationScope private val scope: CoroutineScope, retrofit: Retrofit, ) { 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 ───────────────────────────────────────────────────────── fun observeLikedArtists(): Flow> = - likeDao.observeLikedIdsOfType(LOCAL_USER_ID, ENTITY_ARTIST).map { ids -> + likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_ARTIST).map { ids -> ids.mapNotNull { artistDao.getById(it)?.toDomain() } } fun observeLikedAlbums(): Flow> = - likeDao.observeLikedIdsOfType(LOCAL_USER_ID, ENTITY_ALBUM).map { ids -> + likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_ALBUM).map { ids -> ids.mapNotNull { albumDao.getById(it)?.toDomain() } } fun observeLikedTracks(): Flow> = - likeDao.observeLikedIdsOfType(LOCAL_USER_ID, ENTITY_TRACK).map { ids -> + likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_TRACK).map { ids -> ids.mapNotNull { trackDao.getById(it)?.toDomain() } } fun observeIsLiked(entityType: String, entityId: String): Flow = - 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. */ suspend fun likedTrackIds(): Set = - 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) ── @@ -85,13 +123,15 @@ class LikesRepository @Inject constructor( * if it got enqueued for later replay. */ 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) { likeDao.upsertAll( - listOf(CachedLikeEntity(LOCAL_USER_ID, entityType, entityId)), + listOf(CachedLikeEntity(uid, entityType, entityId)), ) } else { - likeDao.delete(LOCAL_USER_ID, entityType, entityId) + likeDao.delete(uid, entityType, entityId) } // 2. Best-effort REST call; 3. enqueue on failure. val kindPath = serverPathFor(entityType) @@ -125,17 +165,24 @@ class LikesRepository @Inject constructor( * insert so the local set is exactly what the server reports. */ suspend fun refreshIds() { + val uid = currentUserId() val wire = api.ids() val rows = mutableListOf() - rows += wire.artistIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ARTIST, it) } - rows += wire.albumIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_ALBUM, it) } - rows += wire.trackIds.map { CachedLikeEntity(LOCAL_USER_ID, ENTITY_TRACK, it) } - likeDao.replaceAllForUser(LOCAL_USER_ID, rows) + rows += wire.artistIds.map { CachedLikeEntity(uid, ENTITY_ARTIST, it) } + rows += wire.albumIds.map { CachedLikeEntity(uid, ENTITY_ALBUM, it) } + rows += wire.trackIds.map { CachedLikeEntity(uid, ENTITY_TRACK, it) } + likeDao.replaceAllForUser(uid, rows) } companion object { - // TODO(Phase 11): swap for AuthStore.userId.value once auth lands. - const val LOCAL_USER_ID: String = "local" + // Pre-auth fallback discriminator. Drift #576: when no user is + // 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_ALBUM: String = "album" const val ENTITY_TRACK: String = "track"