diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/MainActivity.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/MainActivity.kt index 17839396bf..90151d7cf3 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/MainActivity.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/MainActivity.kt @@ -22,6 +22,8 @@ import com.flipcash.app.billing.BillingClient import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.contacts.LocalContactCoordinator import com.flipcash.app.core.LocalUserManager +import com.flipcash.app.core.media.LocalMediaUrlResolver +import com.flipcash.app.core.media.MediaUrlResolver import com.flipcash.app.core.tipping.LocalTipCoordinator import com.flipcash.app.core.toast.LocalToastController import com.flipcash.app.core.toast.ToastController @@ -142,6 +144,9 @@ class MainActivity : FragmentActivity() { @Inject lateinit var tippingCoordinator: TippingCoordinator + @Inject + lateinit var mediaUrlResolver: MediaUrlResolver + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) handleUncaughtException() @@ -177,6 +182,7 @@ class MainActivity : FragmentActivity() { LocalToastController provides toastController, LocalCoinbaseOnRampController provides coinbaseOnRampController, LocalTipCoordinator provides tippingCoordinator, + LocalMediaUrlResolver provides mediaUrlResolver, LocalUiTesting provides intent.getBooleanExtra(UI_TEST, false), ) { ProvidePermissionChecker(permissionChecker) { diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt index a7d69bd70e..b51a91279b 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt @@ -28,6 +28,7 @@ import com.flipcash.app.core.navigation.destinationRoute import com.flipcash.app.core.ui.NavigationBar import com.flipcash.app.core.ui.rememberNavigationBarState import com.flipcash.app.session.LocalSessionController +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.user.AuthState import com.flipcash.shared.common.ui.ContactAvatar import com.getcode.manager.BottomBarManager @@ -157,5 +158,13 @@ private fun rememberProfileAvatar(): (@Composable (Modifier) -> Unit)? { val picture = profile?.profilePicture ?: return null val displayName = profile?.displayName.orEmpty() - return { modifier -> ContactAvatar(picture, displayName, modifier) } + // The account's own picture, so its blobs are the caller's own — no access context needed. + return { modifier -> + ContactAvatar( + image = picture, + displayName = displayName, + access = BlobAccessContext.Owned, + modifier = modifier, + ) + } } diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/ImageCachePresence.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/ImageCachePresence.kt new file mode 100644 index 0000000000..8c8dcff514 --- /dev/null +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/ImageCachePresence.kt @@ -0,0 +1,45 @@ +package com.flipcash.app.core.media + +import android.content.Context +import coil3.SingletonImageLoader +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.android.qualifiers.ApplicationContext +import dagger.hilt.components.SingletonComponent +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Whether the image loader already holds the bytes filed under a stable cache key. Blob renditions + * are cached by their durable blob id rather than their download URL, so a hit here means the URL + * will never be dereferenced — and an expired one costs nothing. + */ +interface ImageCachePresence { + suspend fun holds(cacheKey: String): Boolean +} + +@Singleton +class CoilImageCachePresence @Inject constructor( + @param:ApplicationContext private val context: Context, +) : ImageCachePresence { + + override suspend fun holds(cacheKey: String): Boolean = withContext(Dispatchers.IO) { + val loader = SingletonImageLoader.get(context) + if (loader.memoryCache?.keys.orEmpty().any { it.key == cacheKey }) { + return@withContext true + } + // openSnapshot takes a read lock on the entry; close it immediately — we only wanted to + // know it exists, the actual read happens inside Coil's own fetcher. + loader.diskCache?.openSnapshot(cacheKey)?.use { true } ?: false + } +} + +@Module +@InstallIn(SingletonComponent::class) +internal abstract class ImageCachePresenceModule { + @Binds + abstract fun bindImageCachePresence(impl: CoilImageCachePresence): ImageCachePresence +} diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/MediaUrlResolver.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/MediaUrlResolver.kt new file mode 100644 index 0000000000..53a33c7d91 --- /dev/null +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/MediaUrlResolver.kt @@ -0,0 +1,107 @@ +package com.flipcash.app.core.media + +import com.flipcash.app.core.time.TimeProvider +import com.flipcash.services.controllers.BlobStorageController +import com.flipcash.services.models.chat.BlobAccessContext +import com.flipcash.services.models.chat.BlobMetadata +import com.flipcash.services.models.chat.MediaItem +import com.flipcash.services.models.chat.MediaItemRendition +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Hands out download URLs for [MediaItem] renditions that are actually usable. + * + * A blob's bytes are immutable but its `download_url` is a short-lived signed URL carrying an + * expiry; the client is expected to call `GetBlobs` for a fresh one once that passes. Because the + * whole [MediaItem] is persisted (profile pictures live in the `user_profiles` table), a stored URL + * is routinely older than its expiry by the time a surface asks for it, and handing that straight + * to the image loader produces a failed load and a placeholder. + * + * Two things stop this from turning into a request per avatar: + * - renditions whose bytes are already cached are left alone, since their URL is never fetched, and + * - re-mints are memoised for the process lifetime and serialised, so a screenful of surfaces + * sharing a rendition costs one call. + * + * Every entry point takes the [BlobAccessContext] the media is being read from, because that is + * what authorizes a re-mint of a blob the caller doesn't own — another user's avatar resolves to + * nothing without it. + * + * [refreshUrlForSize] is the recovery path for a URL that failed anyway — metadata persisted before + * expiry was modelled carries no expiry at all, so it can only be found stale by trying it. + */ +@Singleton +class MediaUrlResolver @Inject constructor( + private val blobStorage: BlobStorageController, + private val cache: ImageCachePresence, + private val time: TimeProvider, +) { + private val mutex = Mutex() + private val reminted = mutableMapOf() + + /** + * A usable download URL for the rendition [MediaItem.renditionForSize] picks for + * [targetLongestSidePx], re-minting first if the stored one has expired. Null when the item has + * no rendition for that size. Falls back to the stored URL if a re-mint fails, so an offline + * client still gets whatever chance the cache gives it. + */ + suspend fun urlForSize( + media: MediaItem, + targetLongestSidePx: Int, + access: BlobAccessContext, + ): String? { + val rendition = media.renditionForSize(targetLongestSidePx) ?: return null + val stored = rendition.blob ?: return null + + reminted[rendition.cacheKey]?.let { return it.downloadUrl } + if (!stored.isDownloadUrlExpired(time.now())) return stored.downloadUrl + if (cache.holds(rendition.cacheKey)) return stored.downloadUrl + + return remint(rendition, access, staleUrl = stored.downloadUrl)?.downloadUrl + ?: stored.downloadUrl + } + + /** + * Re-mints the rendition for [targetLongestSidePx] after [failedUrl] failed to load, returning + * a URL to try instead, or null if the blob could not be resolved. This is the only signal + * available for metadata that carries no expiry — every row persisted before expiry was + * modelled — so a failure is taken to mean the URL was stale. + * + * Passing the URL that actually failed is what keeps this from duplicating work: if another + * caller has already minted a URL past that one, it is returned as-is. + */ + suspend fun refreshUrlForSize( + media: MediaItem, + targetLongestSidePx: Int, + failedUrl: String?, + access: BlobAccessContext, + ): String? { + val rendition = media.renditionForSize(targetLongestSidePx) ?: return null + return remint(rendition, access, staleUrl = failedUrl)?.downloadUrl + } + + /** Drops memoised URLs — they are minted for the signed-in owner and don't outlive the session. */ + suspend fun reset() { + mutex.withLock { reminted.clear() } + } + + // Serialised so a screenful of surfaces resolving the same rendition makes one call, not one + // each. Callers that were already waiting take whatever the winner minted, as long as it isn't + // the [staleUrl] they came in holding. + private suspend fun remint( + rendition: MediaItemRendition, + access: BlobAccessContext, + staleUrl: String?, + ): BlobMetadata? = mutex.withLock { + reminted[rendition.cacheKey] + ?.takeIf { it.downloadUrl != staleUrl } + ?.let { return@withLock it } + + blobStorage.refreshMetadata(listOf(rendition.blobId), access) + .getOrNull() + ?.get(rendition.cacheKey) + ?.also { reminted[rendition.cacheKey] = it } + } +} diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/RememberMediaUrl.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/RememberMediaUrl.kt new file mode 100644 index 0000000000..9be4926596 --- /dev/null +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/media/RememberMediaUrl.kt @@ -0,0 +1,85 @@ +package com.flipcash.app.core.media + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.Stable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.runtime.staticCompositionLocalOf +import com.flipcash.services.models.chat.BlobAccessContext +import com.flipcash.services.models.chat.MediaItem + +val LocalMediaUrlResolver = staticCompositionLocalOf { null } + +/** + * The download URL a blob-backed surface should render, and whether rendering it is hopeless. + * + * [url] starts as the URL stored on the [MediaItem] and is replaced if it needs re-minting, so a + * cached rendition draws on the first frame instead of waiting on a resolve. [hasFailed] only goes + * true once a re-minted URL has failed too — a first failure is treated as a stale URL and retried + * silently, leaving whatever placeholder is on screen in place. + */ +@Stable +class MediaUrlState internal constructor(initialUrl: String?) { + var url: String? by mutableStateOf(initialUrl) + internal set + + var hasFailed: Boolean by mutableStateOf(false) + internal set + + internal var failures by mutableIntStateOf(0) + private set + + /** The URL the last failure was reported against — what a re-mint has to improve on. */ + internal var failedUrl: String? = null + private set + + /** Report that the image loader could not load [url]. */ + fun onLoadFailed() { + failedUrl = url + failures++ + } +} + +/** + * Resolves [media]'s rendition for [targetLongestSidePx] into a URL that can be handed to the image + * loader, keeping it usable as the stored one expires. See [MediaUrlResolver] for why a stored URL + * often isn't. [access] names the surface [media] is being read from — a re-mint of media the + * caller doesn't own resolves to nothing without it. + */ +@Composable +fun rememberMediaUrl( + media: MediaItem?, + targetLongestSidePx: Int, + access: BlobAccessContext, +): MediaUrlState { + val resolver = LocalMediaUrlResolver.current + val state = remember(media, targetLongestSidePx) { + MediaUrlState(media?.urlForSize(targetLongestSidePx)) + } + + LaunchedEffect(state, resolver) { + if (media == null || resolver == null) return@LaunchedEffect + resolver.urlForSize(media, targetLongestSidePx, access)?.let { state.url = it } + } + + LaunchedEffect(state, resolver, state.failures) { + if (state.failures == 0) return@LaunchedEffect + // Only the first failure is worth a re-mint; a fresh URL that also fails is a real error. + if (media == null || resolver == null || state.failures > 1) { + state.hasFailed = true + return@LaunchedEffect + } + val fresh = resolver.refreshUrlForSize(media, targetLongestSidePx, state.failedUrl, access) + if (fresh != null && fresh != state.url) { + state.url = fresh + } else { + state.hasFailed = true + } + } + + return state +} diff --git a/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/media/MediaUrlResolverTest.kt b/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/media/MediaUrlResolverTest.kt new file mode 100644 index 0000000000..6675f5f530 --- /dev/null +++ b/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/media/MediaUrlResolverTest.kt @@ -0,0 +1,173 @@ +package com.flipcash.app.core.media + +import com.flipcash.app.core.time.FakeTimeProvider +import com.flipcash.services.controllers.BlobStorageController +import com.flipcash.services.models.chat.BlobAccessContext +import com.flipcash.services.models.chat.BlobId +import com.flipcash.services.models.chat.BlobMetadata +import com.flipcash.services.models.chat.ImageMetadata +import com.flipcash.services.models.chat.MediaItem +import com.flipcash.services.models.chat.MediaItemRendition +import com.getcode.utils.base58 +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import kotlinx.coroutines.async +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Instant + +class MediaUrlResolverTest { + + private val now = Instant.fromEpochMilliseconds(1_700_000_000_000) + private val time = FakeTimeProvider(now) + private val controller = mockk() + + private var cachedKeys = emptySet() + private val cache = object : ImageCachePresence { + override suspend fun holds(cacheKey: String): Boolean = cacheKey in cachedKeys + } + + private val resolver = MediaUrlResolver(controller, cache, time) + + // Every avatar in these tests belongs to someone else, which is the case that needs a context. + private val access = BlobAccessContext.Profile(listOf(7)) + + private val thumbId = BlobId(byteArrayOf(1)) + private val thumbKey = byteArrayOf(1).base58 + + private fun media(expiresAt: Instant?, url: String = "https://cdn/stored") = MediaItem( + listOf( + MediaItemRendition( + role = MediaItemRendition.Role.THUMBNAIL, + blobId = thumbId, + blob = BlobMetadata( + mimeType = "image/png", + sizeBytes = 1, + downloadUrl = url, + image = ImageMetadata(width = 160, height = 160, blurhash = "abc"), + expiresAtMillis = expiresAt?.toEpochMilliseconds(), + ), + ) + ) + ) + + private fun freshMetadata(url: String) = BlobMetadata( + mimeType = "image/png", + sizeBytes = 1, + downloadUrl = url, + image = ImageMetadata(width = 160, height = 160, blurhash = "abc"), + expiresAtMillis = (now + 10.minutes).toEpochMilliseconds(), + ) + + @Test + fun `a live URL is handed straight back`() = runTest { + val url = resolver.urlForSize(media(expiresAt = now + 10.minutes), 160, access) + + assertEquals("https://cdn/stored", url) + coVerify(exactly = 0) { controller.refreshMetadata(any(), any()) } + } + + @Test + fun `an expired URL is re-minted`() = runTest { + coEvery { controller.refreshMetadata(any(), any()) } returns + Result.success(mapOf(thumbKey to freshMetadata("https://cdn/fresh"))) + + val url = resolver.urlForSize(media(expiresAt = now - 1.minutes), 160, access) + + assertEquals("https://cdn/fresh", url) + coVerify(exactly = 1) { controller.refreshMetadata(listOf(thumbId), any()) } + } + + @Test + fun `an expired URL whose bytes are already cached costs no round trip`() = runTest { + cachedKeys = setOf(thumbKey) + + val url = resolver.urlForSize(media(expiresAt = now - 1.minutes), 160, access) + + assertEquals("https://cdn/stored", url) + coVerify(exactly = 0) { controller.refreshMetadata(any(), any()) } + } + + @Test + fun `a failed re-mint falls back to the stored URL`() = runTest { + coEvery { controller.refreshMetadata(any(), any()) } returns Result.failure(Throwable("offline")) + + val url = resolver.urlForSize(media(expiresAt = now - 1.minutes), 160, access) + + assertEquals("https://cdn/stored", url) + } + + @Test + fun `surfaces sharing a rendition share one re-mint`() = runTest { + coEvery { controller.refreshMetadata(any(), any()) } returns + Result.success(mapOf(thumbKey to freshMetadata("https://cdn/fresh"))) + val item = media(expiresAt = now - 1.minutes) + + val first = async { resolver.urlForSize(item, 160, access) } + val second = async { resolver.urlForSize(item, 160, access) } + + assertEquals("https://cdn/fresh", first.await()) + assertEquals("https://cdn/fresh", second.await()) + coVerify(exactly = 1) { controller.refreshMetadata(any(), any()) } + } + + @Test + fun `refreshUrlForSize re-mints metadata that carries no expiry`() = runTest { + // Rows persisted before expiry was modelled: nothing marks them stale but a failed load. + coEvery { controller.refreshMetadata(any(), any()) } returns + Result.success(mapOf(thumbKey to freshMetadata("https://cdn/fresh"))) + val item = media(expiresAt = null) + + assertEquals("https://cdn/stored", resolver.urlForSize(item, 160, access)) + assertEquals("https://cdn/fresh", resolver.refreshUrlForSize(item, 160, "https://cdn/stored", access)) + } + + @Test + fun `reset drops URLs minted for the previous owner`() = runTest { + coEvery { controller.refreshMetadata(any(), any()) } returns + Result.success(mapOf(thumbKey to freshMetadata("https://cdn/fresh"))) + val item = media(expiresAt = now - 1.minutes) + resolver.urlForSize(item, 160, access) + + resolver.reset() + resolver.urlForSize(item, 160, access) + + coVerify(exactly = 2) { controller.refreshMetadata(any(), any()) } + } + + @Test + fun `a load that failed on an already-superseded URL reuses the mint it missed`() = runTest { + // urlForSize re-mints while a surface is still showing the stale URL; that surface's + // failure must not spend a second call re-minting what it can already be handed. + coEvery { controller.refreshMetadata(any(), any()) } returns + Result.success(mapOf(thumbKey to freshMetadata("https://cdn/fresh"))) + val item = media(expiresAt = now - 1.minutes) + resolver.urlForSize(item, 160, access) + + val retry = resolver.refreshUrlForSize(item, 160, failedUrl = "https://cdn/stored", access) + + assertEquals("https://cdn/fresh", retry) + coVerify(exactly = 1) { controller.refreshMetadata(any(), any()) } + } + + @Test + fun `the caller's access context is what re-mints someone else's media`() = runTest { + // Without it the server omits blobs the caller doesn't own, so the re-mint comes back empty + // and the avatar falls back — the failure this whole path exists to avoid. + coEvery { controller.refreshMetadata(any(), any()) } returns + Result.success(mapOf(thumbKey to freshMetadata("https://cdn/fresh"))) + + resolver.urlForSize(media(expiresAt = now - 1.minutes), 160, access) + + coVerify(exactly = 1) { controller.refreshMetadata(listOf(thumbId), access) } + } + + @Test + fun `an item with no rendition resolves to null`() = runTest { + assertEquals(null, resolver.urlForSize(MediaItem(emptyList()), 160, access)) + assertEquals(null, resolver.refreshUrlForSize(MediaItem(emptyList()), 160, null, access)) + } +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ParticipantAvatar.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ParticipantAvatar.kt index bfc508417f..065e99a21f 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ParticipantAvatar.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ParticipantAvatar.kt @@ -20,7 +20,14 @@ internal fun ParticipantAvatar( ) { when (participant) { is ChatParticipant.Contact -> ContactAvatar(contact = participant.contact, modifier = modifier) - is ChatParticipant.TipUser -> ContactAvatar(userProfile = participant.profile, modifier = modifier) + // The member's own id, not profile.userId: the server sets the id on the chat member and + // leaves it unset inside the nested profile, and without it the picture's expired download + // URL cannot be re-minted. + is ChatParticipant.TipUser -> ContactAvatar( + userProfile = participant.profile, + modifier = modifier, + userId = participant.userId, + ) null -> ContactAvatar(contact = null, modifier = modifier) } } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt index 4a22f139b8..fdbeadda33 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt @@ -9,13 +9,15 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull -import kotlin.test.assertSame import kotlin.test.assertTrue import kotlin.time.Instant /** * The selection bar and the composer takeover are both plain state, so the reducer is where their * rules live: one message selected at a time, and an edit that never costs the user their draft. + * + * Selection is asserted by value, not identity: the reducer re-narrows the bubble's capabilities to + * the windows open now, so what it stores is a copy of the bubble it was handed. */ class ChatMessageActionReducerTest { @@ -52,7 +54,7 @@ class ChatMessageActionReducerTest { ChatViewModel.Event.ToggleMessageSelection(target), ) - assertSame(target, state.selection) + assertEquals(target, state.selection) assertEquals(target.capabilities, state.selectionCapabilities) } @@ -81,7 +83,7 @@ class ChatMessageActionReducerTest { val state = reduce(selected, ChatViewModel.Event.ToggleMessageSelection(second)) - assertSame(second, state.selection) + assertEquals(second, state.selection) } @Test @@ -108,7 +110,7 @@ class ChatMessageActionReducerTest { val state = reduce(selected, ChatViewModel.Event.DeleteMessage(target.messageId)) - assertSame(target, state.selection) + assertEquals(target, state.selection) assertTrue(state.confirmingDelete) } diff --git a/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/blocklist/BlocklistScreenContent.kt b/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/blocklist/BlocklistScreenContent.kt index 92709fbb80..ee42f3cca2 100644 --- a/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/blocklist/BlocklistScreenContent.kt +++ b/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/blocklist/BlocklistScreenContent.kt @@ -33,6 +33,7 @@ import androidx.paging.compose.itemKey import kotlinx.coroutines.delay import com.flipcash.app.core.blocklist.BlockedUserProfile import com.flipcash.features.myaccount.R +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.shared.common.ui.ContactAvatar import com.getcode.theme.CodeTheme import com.getcode.ui.theme.CodeCircularProgressIndicator @@ -113,6 +114,7 @@ private fun BlockedUserRow( ContactAvatar( image = user.profilePicture, displayName = user.name.orEmpty(), + access = BlobAccessContext.profile(user.userId), // Blocked users are shown obscured, per the design. blurred = true, modifier = Modifier diff --git a/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/userprofile/UserProfileScreenContent.kt b/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/userprofile/UserProfileScreenContent.kt index 2bf81fac49..f4855604a8 100644 --- a/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/userprofile/UserProfileScreenContent.kt +++ b/apps/flipcash/features/myaccount/src/main/kotlin/com/flipcash/app/myaccount/internal/userprofile/UserProfileScreenContent.kt @@ -63,6 +63,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.unit.sp import com.flipcash.app.core.util.abbreviatedLink +import com.flipcash.services.models.chat.BlobAccessContext import com.getcode.theme.White05 import com.getcode.theme.extraSmall import com.getcode.ui.core.verticalScrollStateGradient @@ -323,6 +324,8 @@ private fun ProfileHeader( ContactAvatar( image = profilePicture, displayName = displayName, + // This screen is the account's own public profile, so it owns the picture's blobs. + access = BlobAccessContext.Owned, modifier = Modifier .size(96.dp) .clip(CircleShape), diff --git a/apps/flipcash/features/tipping/src/main/kotlin/com/flipcash/app/tipping/internal/components/TipChatRow.kt b/apps/flipcash/features/tipping/src/main/kotlin/com/flipcash/app/tipping/internal/components/TipChatRow.kt index 6aab7b47e2..c98008eed8 100644 --- a/apps/flipcash/features/tipping/src/main/kotlin/com/flipcash/app/tipping/internal/components/TipChatRow.kt +++ b/apps/flipcash/features/tipping/src/main/kotlin/com/flipcash/app/tipping/internal/components/TipChatRow.kt @@ -6,6 +6,7 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.shared.chat.ui.ChatListRow import com.flipcash.shared.chat.ui.ChatRowSubtitle import com.flipcash.shared.chat.ui.ChatRowTrailing @@ -28,6 +29,7 @@ internal fun TipChatRow( ContactAvatar( image = chat.image, displayName = chat.name.orEmpty(), + access = BlobAccessContext.profile(chat.userId), modifier = Modifier .requiredSize(CodeTheme.dimens.staticGrid.x8) .clip(CircleShape), diff --git a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt index 824094b3fc..acb76c7e37 100644 --- a/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt +++ b/apps/flipcash/features/user-profile/src/main/kotlin/com/flipcash/app/userprofile/internal/photo/PhotoSelectionScreen.kt @@ -41,6 +41,7 @@ import com.flipcash.app.core.ui.transitions.sharedBoundsTransition import com.flipcash.app.core.userprofile.UpdateProfileResult import com.flipcash.app.core.userprofile.UpdateProfileStep import com.flipcash.core.R +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.shared.common.ui.ContactAvatar import com.getcode.navigation.flow.rememberFlowNavigator import com.getcode.theme.CodeTheme @@ -183,6 +184,7 @@ private fun PhotoSelectionScreenContent( ContactAvatar( image = state.savedPicture, displayName = state.name, + access = BlobAccessContext.Owned, modifier = Modifier.fillMaxSize(), ) } diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt index 4a14fee51f..f1b96b66d5 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatSummaryMapping.kt @@ -30,6 +30,7 @@ fun ChatSummary.toConversationReference( val other = metadata.members.firstOrNull { it.userId != selfId } return ConversationReference( chatId = metadata.chatId, + userId = other?.userId, displayName = other?.userProfile?.displayName, handle = other?.userProfile?.handle, image = other?.userProfile?.profilePicture, diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ConversationReference.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ConversationReference.kt index 670c74a2ac..4bf9cf1cc9 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ConversationReference.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ConversationReference.kt @@ -1,5 +1,6 @@ package com.flipcash.shared.chat.ui +import com.getcode.opencode.model.core.ID import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.MediaItem import com.flipcash.services.models.nameOrHandle @@ -8,6 +9,12 @@ import kotlin.time.Instant /** Presentation state derived from an existing DM with a contact. */ data class ConversationReference( val chatId: ChatId, + /** + * Counterparty account id, when the chat has a resolved member. Rows need it to re-mint + * [image]'s download URL: the picture is not the caller's, so only that user's profile + * authorizes reading it. + */ + val userId: ID? = null, /** Counterparty display name — used when the row has no separate contact (e.g. tip DMs). */ val displayName: String? = null, /** diff --git a/apps/flipcash/shared/common-ui/src/main/kotlin/com/flipcash/shared/common/ui/ContactAvatar.kt b/apps/flipcash/shared/common-ui/src/main/kotlin/com/flipcash/shared/common/ui/ContactAvatar.kt index 3251205439..2fee4b4565 100644 --- a/apps/flipcash/shared/common-ui/src/main/kotlin/com/flipcash/shared/common/ui/ContactAvatar.kt +++ b/apps/flipcash/shared/common-ui/src/main/kotlin/com/flipcash/shared/common/ui/ContactAvatar.kt @@ -1,5 +1,6 @@ package com.flipcash.shared.common.ui +import android.graphics.Bitmap import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.border @@ -33,11 +34,15 @@ import coil3.asImage import coil3.compose.AsyncImage import coil3.request.ImageRequest import coil3.request.crossfade +import coil3.request.error import coil3.request.placeholder import com.flipcash.app.core.contacts.DeviceContact +import com.flipcash.app.core.media.rememberMediaUrl import com.flipcash.services.models.HandlePrefix import com.flipcash.services.models.UserProfile +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.models.chat.MediaItem +import com.getcode.opencode.model.core.ID import com.getcode.theme.CodeTheme import com.getcode.ui.core.addIf @@ -103,13 +108,22 @@ fun ContactAvatar( } } +/** + * [userId] identifies whose profile this is, which is what authorizes re-minting the picture's + * expired download URL. It defaults to the profile's own id and only needs passing where the + * caller holds a more reliable one — a chat member's profile, for instance, arrives with the id + * on the member rather than inside the nested profile. + */ @Composable fun ContactAvatar( userProfile: UserProfile, modifier: Modifier = Modifier, + userId: ID? = userProfile.userId, ) { ProfileAvatar( image = userProfile.profilePicture, + // The picture belongs to this profile, so the profile is what authorizes re-minting it. + access = BlobAccessContext.profile(userId), modifier = modifier, fallback = { UnknownContactAvatar(includeBorder = true) }, ) @@ -122,16 +136,22 @@ fun ContactAvatar( * display sizes) so the image is never grainy or over-fetched, and bridges the load with the * item's BlurHash plus any already-cached smaller rendition. Falls back to [displayName]'s * initials when there's no picture. + * + * [access] is the surface the picture is being read from. A stored download URL expires, and + * re-minting one for a blob the caller doesn't own is only authorized by naming that surface — + * [BlobAccessContext.profile] covers every avatar, given the id of the profile it belongs to. */ @Composable fun ContactAvatar( image: MediaItem?, displayName: String, + access: BlobAccessContext, modifier: Modifier = Modifier, blurred: Boolean = false, ) { ProfileAvatar( image = image, + access = access, modifier = modifier, blurred = blurred, fallback = { InitialsText(displayName) }, @@ -141,6 +161,7 @@ fun ContactAvatar( @Composable private fun ProfileAvatar( image: MediaItem?, + access: BlobAccessContext, modifier: Modifier, blurred: Boolean = false, fallback: @Composable BoxWithConstraintsScope.() -> Unit, @@ -150,23 +171,21 @@ private fun ProfileAvatar( Brush.linearGradient(CodeTheme.colors.contactAvatar.colors) ) ) { + // Decoded once for the whole avatar, not just the loading state: it is the placeholder + // while the rendition downloads, what stays on screen while a stale URL is re-minted, and + // what's left if the load never succeeds — anything is a better likeness of the person than + // the grey Person glyph. + // Kept as the decoded Bitmap rather than either wrapper: the two consumers want different + // ones (Compose's ImageBitmap here, Coil's Image in the request), and decoding is the + // expensive half. + val blurBitmap = remember(image) { + BlurHash.decode(image?.blurhash(), width = 24, height = 24) + } if (blurred) { // Blocked users are shown intentionally obscured — render the media item's self-contained // BlurHash preview instead of the real image, so the avatar stays blurred on every API // level (Modifier.blur needs API 31+) without ever fetching the sharp photo. - val blurBitmap = remember(image) { - BlurHash.decode(image?.blurhash(), width = 24, height = 24)?.asImageBitmap() - } - if (blurBitmap != null) { - Image( - bitmap = blurBitmap, - contentDescription = null, - modifier = Modifier.matchParentSize(), - contentScale = ContentScale.Crop, - ) - } else { - fallback() - } + BlurHashOr(blurBitmap, fallback) return@BoxWithConstraints } // Pick the rendition by the avatar's actual pixel size — the longest bounded side of the @@ -176,9 +195,13 @@ private fun ProfileAvatar( val h = if (constraints.hasBoundedHeight) constraints.maxHeight else 0 maxOf(w, h).takeIf { it > 0 } ?: Int.MAX_VALUE } - val photoUri = remember(image, targetPx) { image?.urlForSize(targetPx) } + // Not `image.urlForSize(targetPx)` directly: the stored download URL expires, and a + // persisted profile picture is routinely past it by the time it's rendered. The resolver + // hands back a re-minted URL when that's the case, and again if a load fails anyway. + val media = rememberMediaUrl(image, targetPx, access) + val photoUri = media.url if (image != null && photoUri != null) { - var isError by rememberSaveable(photoUri) { mutableStateOf(false) } + val isError = media.hasFailed if (!isError) { val context = LocalContext.current // Two progressively better placeholders bridge the load so we never flash a blank @@ -187,9 +210,7 @@ private fun ProfileAvatar( // 2. the next-smaller rendition — if another surface already cached it (e.g. the // list loaded the 160 this 320 avatar sits above), Coil shows it immediately // (see placeholderMemoryCacheKey) and upgrades in place. - val blurHash = remember(image) { - BlurHash.decode(image.blurhash(), width = 24, height = 24)?.asImage() - } + val blurHash = remember(blurBitmap) { blurBitmap?.asImage() } // Cache identity is the durable blob id, NOT the download URL: the server re-mints // and expires `download_url` on every fetch, so a URL-keyed cache misses on the next // fetch even though the bytes are immutable — every load would re-download and flash @@ -211,7 +232,14 @@ private fun ProfileAvatar( .memoryCacheKey(cacheKey) .diskCacheKey(cacheKey) .apply { - blurHash?.let { placeholder(it) } + blurHash?.let { + placeholder(it) + // `placeholder` only covers the load itself. A failed load draws + // the error drawable, so without this the avatar blanks the moment + // an expired URL 403s — including for the whole re-mint round trip, + // which is exactly when the BlurHash is worth the most. + error(it) + } previewKey?.let { placeholderMemoryCacheKey(it) } } .build() @@ -224,18 +252,41 @@ private fun ProfileAvatar( // AsyncImage infers the Coil request scale from this, so the request is fine. contentScale = ContentScale.Crop, contentDescription = null, - onError = { isError = true }, + onError = { media.onLoadFailed() }, ) } if (isError) { - fallback() + BlurHashOr(blurBitmap, fallback) } } else { - fallback() + BlurHashOr(blurBitmap, fallback) } } } +/** + * The [blurBitmap] preview if the media carried a BlurHash, otherwise [fallback]. Preferred over + * the initials/Person fallback wherever a hash exists: it is the real photo's colours, and it is + * already in hand — no fetch, no expiry, nothing to fail. + */ +@Composable +private fun BoxWithConstraintsScope.BlurHashOr( + blurBitmap: Bitmap?, + fallback: @Composable BoxWithConstraintsScope.() -> Unit, +) { + val bitmap = remember(blurBitmap) { blurBitmap?.asImageBitmap() } + if (bitmap != null) { + Image( + bitmap = bitmap, + contentDescription = null, + modifier = Modifier.matchParentSize(), + contentScale = ContentScale.Crop, + ) + } else { + fallback() + } +} + @Composable private fun UnknownContactAvatar( includeBorder: Boolean, diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index 17a3adcd32..02805ef5ca 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -22,6 +22,7 @@ import coil3.request.ImageRequest import coil3.request.SuccessResult import coil3.request.allowHardware import coil3.toBitmap +import com.flipcash.app.core.media.MediaUrlResolver import com.flipcash.app.auth.AuthManager import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.contacts.ContactResolver @@ -41,6 +42,7 @@ import com.flipcash.services.models.NotificationCategory import com.flipcash.services.models.NotificationPayload import com.flipcash.services.models.PushChatMetadata import com.flipcash.services.models.Substitution +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.models.chat.ChatType import com.flipcash.services.user.UserManager import com.flipcash.shared.notifications.R @@ -82,6 +84,9 @@ class NotificationService : FirebaseMessagingService(), @Inject lateinit var pushController: PushController + @Inject + lateinit var mediaUrlResolver: MediaUrlResolver + @Inject lateinit var notificationManager: NotificationManagerCompat @@ -267,9 +272,18 @@ class NotificationService : FirebaseMessagingService(), // the tiny 32px one looks grainy on the notification's person icon. val avatarPx = resources.getDimensionPixelSize(android.R.dimen.notification_large_icon_width) val avatar = e164?.let { resolveContactPhoto(it) } - ?: member?.userProfile?.profilePicture - ?.urlForSize(avatarPx) - ?.let { loadRemoteAvatar(it) } + ?: member?.userProfile?.profilePicture?.let { picture -> + // Through the resolver, not `picture.urlForSize` — the stored download URL expires + // and the profile it came from may have been persisted days ago. + mediaUrlResolver.urlForSize( + media = picture, + targetLongestSidePx = avatarPx, + // The counterparty's picture, so their profile is what authorizes a re-mint. + access = BlobAccessContext.profile(member?.userId), + )?.let { url -> + loadRemoteAvatar(url, picture.cacheKeyForSize(avatarPx)) + } + } trace( tag = "NotificationService", @@ -317,11 +331,17 @@ class NotificationService : FirebaseMessagingService(), * notification. Returns `null` on timeout or failure (the notification then * posts with the name monogram). */ - private suspend fun loadRemoteAvatar(url: String): Bitmap? = + private suspend fun loadRemoteAvatar(url: String, cacheKey: String?): Bitmap? = withTimeoutOrNull(AVATAR_FETCH_TIMEOUT_MS.milliseconds) { runCatching { val request = ImageRequest.Builder(this@NotificationService) .data(url) + // Keyed on the durable blob id like the in-app avatars, so a notification hits + // the rendition they already cached instead of re-downloading under a URL that + // is different on every mint. + .apply { + cacheKey?.let { memoryCacheKey(it); diskCacheKey(it) } + } .allowHardware(false) // notification icons require a software bitmap .build() val result = SingletonImageLoader.get(this@NotificationService).execute(request) diff --git a/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt b/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt index a18357c4c1..8e4685eadc 100644 --- a/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt +++ b/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt @@ -8,6 +8,7 @@ import com.flipcash.app.billing.BillingClient import com.flipcash.app.blocklist.BlocklistCoordinator import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.blob.BlobStorageCoordinator +import com.flipcash.app.core.media.MediaUrlResolver import com.flipcash.services.models.chat.ChatType import com.flipcash.shared.chat.ChatCoordinator import com.flipcash.app.core.bill.Scannable @@ -110,6 +111,7 @@ class RealSessionController @Inject constructor( private val chatCoordinator: ChatCoordinator, private val blocklistCoordinator: BlocklistCoordinator, private val blobStorageCoordinator: BlobStorageCoordinator, + private val mediaUrlResolver: MediaUrlResolver, networkObserver: NetworkConnectivityListener, featureFlagController: FeatureFlagController, appSettingsCoordinator: AppSettingsCoordinator, @@ -191,6 +193,8 @@ class RealSessionController @Inject constructor( depositDelegate.cancelSweep() scope.launch { contactCoordinator.reset() } scope.launch { chatCoordinator.teardown() } + // Blob download URLs are minted for the signed-in owner. + scope.launch { mediaUrlResolver.reset() } stateHolder.reset() } diff --git a/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerEventRoutingTest.kt b/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerEventRoutingTest.kt index 2226ad184d..735eb77174 100644 --- a/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerEventRoutingTest.kt +++ b/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerEventRoutingTest.kt @@ -121,6 +121,7 @@ class SessionControllerEventRoutingTest { chatCoordinator = mockk(relaxed = true), blocklistCoordinator = mockk(relaxed = true), blobStorageCoordinator = mockk(relaxed = true), + mediaUrlResolver = mockk(relaxed = true), featureFlagController = featureFlagController, appSettingsCoordinator = appSettingsCoordinator, dispatchers = dispatchers, diff --git a/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerGiftCardErrorTest.kt b/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerGiftCardErrorTest.kt index 0daee54d4a..51cd8374b3 100644 --- a/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerGiftCardErrorTest.kt +++ b/apps/flipcash/shared/session/src/test/kotlin/com/flipcash/app/session/internal/SessionControllerGiftCardErrorTest.kt @@ -147,6 +147,7 @@ class SessionControllerGiftCardErrorTest { chatCoordinator = mockk(relaxed = true), blocklistCoordinator = mockk(relaxed = true), blobStorageCoordinator = mockk(relaxed = true), + mediaUrlResolver = mockk(relaxed = true), featureFlagController = mockk(relaxed = true), appSettingsCoordinator = mockk(relaxed = true), dispatchers = dispatchers, diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 035cc07257..13da67359c 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -61,7 +61,7 @@ grpc = "1.83.1" grpc-okhttp = "1.83.1" grpc-kotlin = "1.5.0" protobuf = "4.36.0" -protovalidate-kt = "0.1.1" +protovalidate-kt = "0.1.2" # Generated client SDKs for the two backend contracts. Separate versions on purpose: the # protos are independent (flipcash2 does not import ocp), so the packages move on their own @@ -69,7 +69,7 @@ protovalidate-kt = "0.1.1" # 0.3.0 is the first release of either package to ship R8 keep rules for its generated # messages, which is what lets proguard-rules.pro drop its own. ocp-client-protocol = "0.3.0" -flipcash2-client-protocol = "0.4.0" +flipcash2-client-protocol = "0.4.1" # The Android port is the ONLY libphonenumber this app depends on, deliberately. Google's # `com.googlecode` artifact used to sit alongside it; the two ship separate copies of the metadata, diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt index 222401e5ee..bdd3675ec9 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/BlobStorageController.kt @@ -4,11 +4,14 @@ import com.flipcash.services.BlobUploader import com.flipcash.services.models.BlobNotReadyException import com.flipcash.services.models.BlobRejectedException import com.flipcash.services.models.blob.UploadPolicy +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.models.chat.BlobId +import com.flipcash.services.models.chat.BlobMetadata import com.flipcash.services.models.chat.BlobState import com.flipcash.services.repository.BlobStorageRepository import com.flipcash.services.user.UserManager import com.getcode.ed25519.Ed25519 +import com.getcode.utils.base58 import kotlinx.coroutines.delay import javax.inject.Inject import kotlin.time.Duration @@ -58,11 +61,38 @@ class BlobStorageController @Inject constructor( return awaitReady(reservation.blobId, owner) } + /** + * Re-resolves [blobIds] to freshly minted [BlobMetadata] — the recovery `DownloadUrl.expires_at` + * calls for. A blob's bytes are immutable but its `download_url` is per-fetch and expiring, so + * any metadata that has been held a while (persisted profile pictures, chat media) needs this + * before its URL can be fetched again. + * + * Returns only the ids that came back READY, keyed by base58 blob id — ids still processing or + * rejected are simply absent, leaving the caller's existing metadata in place. + * + * [context] names the surface the blobs are being read from. It is what authorizes ids the + * caller does not own — another user's avatar resolves only through + * [BlobAccessContext.Profile], chat media only through [BlobAccessContext.Chat] — and without + * it the server omits them from the response rather than failing, so a wrong context looks + * exactly like a blob that isn't ready yet. + */ + suspend fun refreshMetadata( + blobIds: List, + context: BlobAccessContext, + ): Result> { + if (blobIds.isEmpty()) return Result.success(emptyMap()) + val owner = owner() ?: return noAccount() + return repository.getBlobs(blobIds, owner, context).map { blobs -> + blobs.filterIsInstance() + .associate { it.id.bytes.base58 to it.metadata } + } + } + private suspend fun awaitReady(blobId: BlobId, owner: Ed25519.KeyPair): Result { var elapsed: Duration = Duration.ZERO while (elapsed < POLL_TIMEOUT) { // A single-id query resolves to at most one blob, so first() is the one we asked for. - val blob = repository.getBlobs(listOf(blobId), owner) + val blob = repository.getBlobs(listOf(blobId), owner, BlobAccessContext.Owned) .getOrElse { return Result.failure(it) } .firstOrNull() diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapper.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapper.kt index 530805973f..fc5b4ed8f8 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapper.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapper.kt @@ -22,7 +22,12 @@ class ChatMetadataMapper @Inject constructor( members = from.membersList.map { member -> ChatMember( userId = member.userId.toId(), - userProfile = userProfileMapper.map(member.userProfile), + // The server sets the id on the member and usually not again inside the nested + // profile, and this profile is by definition that member's. Leaving it null + // costs callers the id that authorizes re-minting the profile picture's + // download URL, so the avatar can never recover once the stored URL expires. + userProfile = userProfileMapper.map(member.userProfile) + .let { if (it.userId == null) it.copy(userId = member.userId.toId()) else it }, pointers = member.pointersList.map { it.toPointer() }, ) }, diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/BlobStorageApi.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/BlobStorageApi.kt index a36755cb07..92a664e084 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/BlobStorageApi.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/BlobStorageApi.kt @@ -5,7 +5,10 @@ import com.codeinc.flipcash.gen.blob.v1.BlobStorageService as RpcBlobStorageServ import com.codeinc.flipcash.gen.blob.v1.Model import com.codeinc.flipcash.gen.blob.v1.validate import com.flipcash.services.internal.annotations.FlipcashManagedChannel +import com.flipcash.services.internal.network.extensions.asChatId +import com.flipcash.services.internal.network.extensions.asUserId import com.flipcash.services.internal.network.extensions.authenticate +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.models.chat.BlobId import com.getcode.ed25519.Ed25519 import com.getcode.opencode.internal.network.core.GrpcApi @@ -81,6 +84,7 @@ internal class BlobStorageApi @Inject constructor( suspend fun getBlobs( blobIds: List, owner: Ed25519.KeyPair, + context: BlobAccessContext, ): RpcBlobStorageService.GetBlobsResponse { val request = RpcBlobStorageService.GetBlobsRequest.newBuilder() .setBlobIds( @@ -88,6 +92,9 @@ internal class BlobStorageApi @Inject constructor( .addAllBlobIds(blobIds.map { it.toProto() }) ) .apply { setAuth(authenticate(owner)) } + // Omitted for Owned: the server resolves the caller's own blobs without one, and a + // scope the caller can't claim would only narrow the read. + .apply { context.toProto()?.let { setContext(it) } } .build() request.validate().orThrow() @@ -99,4 +106,12 @@ internal class BlobStorageApi @Inject constructor( private fun BlobId.toProto(): Model.BlobId = Model.BlobId.newBuilder().setValue(bytes.toByteString()).build() + + private fun BlobAccessContext.toProto(): Model.AccessContext? = when (this) { + BlobAccessContext.Owned -> null + is BlobAccessContext.Profile -> + Model.AccessContext.newBuilder().setProfile(userId.asUserId()).build() + is BlobAccessContext.Chat -> + Model.AccessContext.newBuilder().setChat(chatId.asChatId()).build() + } } diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt index 70bc3912f6..0bccc3a3cc 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt @@ -214,6 +214,12 @@ internal fun com.codeinc.flipcash.gen.blob.v1.Model.BlobMetadata.toBlobMetadata( sizeBytes = sizeBytes, downloadUrl = downloadUrl.url, image = if (hasImage()) image.toImageMetadata() else null, + // Carried, not dropped: this metadata gets persisted (profile pictures live in + // `user_profiles`), so without the expiry a stored copy hands Coil a URL that has long + // since 403'd, with nothing able to tell that it should re-resolve the id first. + expiresAtMillis = downloadUrl.takeIf { it.hasExpiresAt() } + ?.expiresAt + ?.let { it.seconds * 1_000 + it.nanos / 1_000_000 }, ) } @@ -377,7 +383,12 @@ internal fun ChatModel.Metadata.toChatMetadata(): ChatMetadata { phoneNumber = phoneNumber.value.takeIf { it.isNotEmpty() }?.let { VerifiableContactMethod(it, verified = true) }, email = emailAddress.value.takeIf { it.isNotEmpty() }?.let { VerifiableContactMethod(it, verified = true) }, profilePicture = if (hasProfilePicture()) profilePicture.toMediaItem() else null, - userId = if (hasUserId()) userId.toId() else null, + // Falls back to the member's own id: the server sets it on the member but + // usually not again inside the nested profile, and this profile is by + // definition that member's. Dropping it here leaves callers unable to name + // the profile that authorizes re-minting the picture's download URL, so the + // avatar can never recover once the stored URL expires. + userId = if (hasUserId()) userId.toId() else member.userId.toId(), username = if (hasUsername()) username.value else null, ) }, diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/services/BlobStorageService.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/services/BlobStorageService.kt index cee1b0696e..3d19b913cc 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/services/BlobStorageService.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/services/BlobStorageService.kt @@ -12,6 +12,7 @@ import com.flipcash.services.models.GetUploadPolicyError import com.flipcash.services.models.InitiateExternalUploadError import com.flipcash.services.models.blob.UploadPolicy import com.flipcash.services.models.blob.UploadReservation +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.models.chat.BlobId import com.flipcash.services.models.chat.BlobState import com.flipcash.services.models.chat.BlobStatus @@ -101,8 +102,9 @@ internal class BlobStorageService @Inject constructor( suspend fun getBlobs( blobIds: List, owner: Ed25519.KeyPair, + context: BlobAccessContext, ): Result> { - return runCatching { api.getBlobs(blobIds, owner) } + return runCatching { api.getBlobs(blobIds, owner, context) } .foldWithSuppression( onSuccess = { response -> when (response.result) { diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/repositories/InternalBlobStorageRepository.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/repositories/InternalBlobStorageRepository.kt index 95dfba528d..67bf8d0069 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/repositories/InternalBlobStorageRepository.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/repositories/InternalBlobStorageRepository.kt @@ -3,6 +3,7 @@ package com.flipcash.services.internal.repositories import com.flipcash.services.internal.network.services.BlobStorageService import com.flipcash.services.models.blob.UploadPolicy import com.flipcash.services.models.blob.UploadReservation +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.models.chat.BlobId import com.flipcash.services.models.chat.BlobState import com.flipcash.services.models.chat.BlobStatus @@ -34,6 +35,7 @@ internal class InternalBlobStorageRepository( override suspend fun getBlobs( blobIds: List, owner: Ed25519.KeyPair, - ): Result> = service.getBlobs(blobIds, owner) + context: BlobAccessContext, + ): Result> = service.getBlobs(blobIds, owner, context) .onFailure { ErrorUtils.handleError(it) } } diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/BlobAccessContext.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/BlobAccessContext.kt new file mode 100644 index 0000000000..046d7c8a51 --- /dev/null +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/BlobAccessContext.kt @@ -0,0 +1,35 @@ +package com.flipcash.services.models.chat + +import com.getcode.opencode.model.core.ID + +/** + * The surface a caller is reading blobs from, used to authorize `GetBlobs`. + * + * Blobs the caller owns resolve on their own, but an id the caller does *not* own is treated as + * unauthorized and silently omitted from the response unless a context that grants it is supplied. + * Re-minting another user's avatar therefore fails closed — an empty response, not an error — so + * every re-mint of someone else's media has to name the surface it is being read from. + */ +sealed interface BlobAccessContext { + /** The caller owns these blobs (their own profile picture, their own uploads). */ + data object Owned : BlobAccessContext + + /** + * Read from [userId]'s public profile. Grants only the renditions of that user's *current* + * profile picture — a superseded picture stops resolving through it. + */ + data class Profile(val userId: ID) : BlobAccessContext + + /** Read from within [chatId]. Granted iff the caller is a member and the blob was shared into it. */ + data class Chat(val chatId: ChatId) : BlobAccessContext + + companion object { + /** + * [Profile] for [userId], falling back to [Owned] when the id isn't known. The fallback is + * for surfaces that hold a picture without the profile it belongs to; it resolves the + * caller's own blobs and quietly resolves nothing for anyone else's, which is the same + * outcome as passing no context at all. + */ + fun profile(userId: ID?): BlobAccessContext = userId?.let(::Profile) ?: Owned + } +} diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/BlobMetadata.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/BlobMetadata.kt index aee4821ecd..e8c716ed51 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/BlobMetadata.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/BlobMetadata.kt @@ -1,6 +1,9 @@ package com.flipcash.services.models.chat import android.os.Parcelable +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant import kotlinx.parcelize.Parcelize import kotlinx.serialization.Serializable @@ -11,4 +14,40 @@ data class BlobMetadata( val sizeBytes: Long, val downloadUrl: String, val image: ImageMetadata?, -): Parcelable + /** + * When [downloadUrl] stops working, as epoch milliseconds — null for metadata that predates + * this field (already-persisted rows) or a server that didn't send one. + * + * Every other field here is intrinsic to the immutable bytes; this one is not. The server + * mints [downloadUrl] per fetch and expires it, so a metadata copy that outlives its expiry — + * and these are persisted, see `user_profiles.profile_picture_json` — carries a URL that 403s. + * Past this instant the id must be re-resolved through `GetBlobs` for a fresh URL. + * + * Stored as millis rather than an `Instant` so the type stays Parcelable and its persisted + * JSON stays a primitive. + */ + val expiresAtMillis: Long? = null, +): Parcelable { + + val expiresAt: Instant? + get() = expiresAtMillis?.let { Instant.fromEpochMilliseconds(it) } + + /** + * Whether [downloadUrl] is past — or within [margin] of — its expiry at [now], and so must be + * re-minted before use. Metadata carrying no expiry is treated as usable: it either predates + * the field or the server declined to bound it, and failing closed there would re-resolve + * every blob on every load. + */ + fun isDownloadUrlExpired(now: Instant, margin: Duration = EXPIRY_MARGIN): Boolean { + val expiry = expiresAt ?: return false + return now + margin >= expiry + } + + companion object { + /** + * Treat a URL as already dead this far ahead of its stated expiry, so one that would die + * mid-download doesn't produce a load error we'd have to recover from. + */ + val EXPIRY_MARGIN: Duration = 30.seconds + } +} diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItem.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItem.kt index bfc7261ad5..105ce557a3 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItem.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItem.kt @@ -1,7 +1,7 @@ package com.flipcash.services.models.chat import android.os.Parcelable -import com.getcode.utils.base58 +import kotlin.time.Instant import kotlinx.parcelize.Parcelize import kotlinx.serialization.Serializable @@ -91,6 +91,31 @@ data class MediaItem( fun blurhash(): String? = renditions.firstNotNullOfOrNull { it.blob?.image?.blurhash?.takeIf(String::isNotEmpty) } + /** + * The rendition [renditionForSize] resolves to, if its download URL has expired at [now] and + * so needs re-minting through `GetBlobs` before it can be fetched. Null when the URL is still + * good, when there is no rendition for the size, or when the metadata carries no expiry. + */ + fun expiredRenditionForSize(targetLongestSidePx: Int, now: Instant): MediaItemRendition? = + renditionForSize(targetLongestSidePx)?.takeIf { it.isDownloadUrlExpired(now) } + + /** + * A copy with every rendition whose [MediaItemRendition.cacheKey] appears in [fresh] carrying + * that re-minted metadata instead. Renditions absent from [fresh] are left untouched, so a + * partial refresh (only the sizes a surface actually needed) is safe to apply. + * + * Keyed by the base58 cache key rather than [BlobId]: BlobId wraps a ByteArray, so its + * equality is referential and it cannot be used as a map key. + */ + fun withRefreshedBlobs(fresh: Map): MediaItem { + if (fresh.isEmpty()) return this + return copy( + renditions = renditions.map { rendition -> + fresh[rendition.cacheKey]?.let { rendition.copy(blob = it) } ?: rendition + } + ) + } + /** Available, non-ORIGINAL renditions paired with their longest side, in list order. */ private fun sizedRenditions(): List> = renditions @@ -108,9 +133,5 @@ data class MediaItem( /** Longest image side (px) of a rendition, or null if it has no image dimensions. */ private val MediaItemRendition.longestSide: Int? get() = blob?.image?.let { maxOf(it.width, it.height) } - - /** Stable, URL-independent cache key for a rendition — its durable blob id, base58-encoded. */ - private val MediaItemRendition.cacheKey: String - get() = blobId.bytes.base58 } } diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItemRendition.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItemRendition.kt index f355a5bfe8..00ee018398 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItemRendition.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/MediaItemRendition.kt @@ -1,6 +1,8 @@ package com.flipcash.services.models.chat import android.os.Parcelable +import com.getcode.utils.base58 +import kotlin.time.Instant import kotlinx.parcelize.Parcelize import kotlinx.serialization.Serializable @@ -17,4 +19,17 @@ data class MediaItemRendition( DISPLAY, THUMBNAIL, } + + /** + * Stable, URL-independent image-cache key — the durable blob id, base58-encoded. The bytes a + * blob id addresses never change, while `download_url` is re-minted per fetch, so keying a + * cache on the URL misses every time even though the bytes are already held. + */ + // Computed, not a stored property: a backing field here would be written by both Parcelize + // and the persisted JSON for a value fully derived from blobId. + val cacheKey: String + get() = blobId.bytes.base58 + + /** Whether this rendition's download URL needs re-minting before use at [now]. */ + fun isDownloadUrlExpired(now: Instant): Boolean = blob?.isDownloadUrlExpired(now) ?: false } diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/repository/BlobStorageRepository.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/repository/BlobStorageRepository.kt index 3ad63c036a..05c6675946 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/repository/BlobStorageRepository.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/repository/BlobStorageRepository.kt @@ -2,6 +2,7 @@ package com.flipcash.services.repository import com.flipcash.services.models.blob.UploadPolicy import com.flipcash.services.models.blob.UploadReservation +import com.flipcash.services.models.chat.BlobAccessContext import com.flipcash.services.models.chat.BlobId import com.flipcash.services.models.chat.BlobState import com.flipcash.services.models.chat.BlobStatus @@ -18,5 +19,9 @@ interface BlobStorageRepository { suspend fun completeExternalUpload(blobId: BlobId, owner: Ed25519.KeyPair): Result - suspend fun getBlobs(blobIds: List, owner: Ed25519.KeyPair): Result> + suspend fun getBlobs( + blobIds: List, + owner: Ed25519.KeyPair, + context: BlobAccessContext, + ): Result> } diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt index a7e7cfcad8..5c6fc3f3c9 100644 --- a/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/controllers/BlobStorageControllerTest.kt @@ -81,7 +81,7 @@ class BlobStorageControllerTest { fun `upload returns the blob id once READY`() = runTest { stubOwner() happyPathStubs() - coEvery { repository.getBlobs(any(), any()) } returns Result.success(listOf(readyBlob())) + coEvery { repository.getBlobs(any(), any(), any()) } returns Result.success(listOf(readyBlob())) val result = controller.upload(byteArrayOf(1, 2, 3), "image/png") @@ -92,7 +92,7 @@ class BlobStorageControllerTest { fun `upload polls until the blob becomes READY`() = runTest { stubOwner() happyPathStubs() - coEvery { repository.getBlobs(any(), any()) } returnsMany listOf( + coEvery { repository.getBlobs(any(), any(), any()) } returnsMany listOf( // Non-terminal polls resolve to an empty list; the controller keeps polling. Result.success(emptyList()), Result.success(emptyList()), @@ -102,14 +102,14 @@ class BlobStorageControllerTest { val result = controller.upload(byteArrayOf(1, 2, 3), "image/png") assertEquals(blobId, result.getOrNull()) - coVerify(exactly = 3) { repository.getBlobs(any(), any()) } + coVerify(exactly = 3) { repository.getBlobs(any(), any(), any()) } } @Test fun `upload fails with BlobRejectedException when the blob is REJECTED`() = runTest { stubOwner() happyPathStubs() - coEvery { repository.getBlobs(any(), any()) } returns Result.success(listOf(rejectedBlob())) + coEvery { repository.getBlobs(any(), any(), any()) } returns Result.success(listOf(rejectedBlob())) val result = controller.upload(byteArrayOf(1, 2, 3), "image/png") @@ -121,7 +121,7 @@ class BlobStorageControllerTest { stubOwner() happyPathStubs() // Never terminal — every poll resolves to an empty list until the timeout trips. - coEvery { repository.getBlobs(any(), any()) } returns Result.success(emptyList()) + coEvery { repository.getBlobs(any(), any(), any()) } returns Result.success(emptyList()) val result = controller.upload(byteArrayOf(1, 2, 3), "image/png") @@ -161,7 +161,7 @@ class BlobStorageControllerTest { coEvery { uploader.upload(any(), any(), any()) } returns Result.success(Unit) coEvery { repository.completeExternalUpload(any(), any()) } returns Result.failure(RuntimeException("complete failed")) - coEvery { repository.getBlobs(any(), any()) } returns Result.success(listOf(readyBlob())) + coEvery { repository.getBlobs(any(), any(), any()) } returns Result.success(listOf(readyBlob())) val result = controller.upload(byteArrayOf(1, 2, 3), "image/png") diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapperTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapperTest.kt index c438a2db95..0833759ccd 100644 --- a/services/flipcash/src/test/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapperTest.kt +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/domain/ChatMetadataMapperTest.kt @@ -111,6 +111,31 @@ class ChatMetadataMapperTest { assertEquals(5L, result.members[0].pointers[0].value) } + @Test + fun `member profile takes the member's user id when the nested profile omits it`() { + val result = mapper.map(metadata { addMembers(member(userIdByte = 9)) }) + assertEquals( + ByteArray(16) { 9 }.toList(), + result.members[0].userProfile.userId, + ) + } + + @Test + fun `member profile keeps its own user id when the server sets one`() { + val withProfileId = ChatModel.Member.newBuilder(member(userIdByte = 9)) + .setUserProfile( + ProfileModel.UserProfile.newBuilder() + .setDisplayName("User") + .setUserId(userId(3)) + ) + .build() + val result = mapper.map(metadata { addMembers(withProfileId) }) + assertEquals( + ByteArray(16) { 3 }.toList(), + result.members[0].userProfile.userId, + ) + } + @Test fun `maps last message when present`() { val result = mapper.map(metadata { setLastMessage(message(text = "hey")) }) diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/api/BlobAccessContextValidationTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/api/BlobAccessContextValidationTest.kt new file mode 100644 index 0000000000..d1a9822c52 --- /dev/null +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/api/BlobAccessContextValidationTest.kt @@ -0,0 +1,50 @@ +package com.flipcash.services.internal.network.api + +import com.codeinc.flipcash.gen.blob.v1.Model +import com.codeinc.flipcash.gen.blob.v1.validate +import com.flipcash.services.internal.network.extensions.asChatId +import com.flipcash.services.internal.network.extensions.asUserId +import com.flipcash.services.models.chat.ChatId +import dev.bmcreations.protovalidate.ValidationResult +import org.junit.Test +import kotlin.test.assertEquals + +/** + * `AccessContext.scope` is a oneof whose arms are each declared `required`, which the generated + * validator got wrong until protovalidate-kt 0.1.2: it asserted every arm was the selected one, so + * whichever arm you set, the others failed as `value is required` and no context could be sent. + * These pin the behaviour the client relies on — [BlobStorageApi.getBlobs] validates the request + * with the context already attached. + */ +class BlobAccessContextValidationTest { + + private fun userId(byte: Byte = 2) = ByteArray(16) { byte }.toList() + + private fun chatId(byte: Byte = 1) = ChatId(ByteArray(32) { byte }) + + @Test + fun `a profile scope validates`() { + val context = Model.AccessContext.newBuilder() + .setProfile(userId().asUserId()) + .build() + + assertEquals(ValidationResult.Valid, context.validate()) + } + + @Test + fun `a chat scope validates`() { + val context = Model.AccessContext.newBuilder() + .setChat(chatId().asChatId()) + .build() + + assertEquals(ValidationResult.Valid, context.validate()) + } + + @Test + fun `an unset scope does not validate`() { + val context = Model.AccessContext.newBuilder().build() + + val result = context.validate() + assertEquals(true, result is ValidationResult.Invalid) + } +} diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/extensions/ChatMetadataExtensionTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/extensions/ChatMetadataExtensionTest.kt new file mode 100644 index 0000000000..508d6ad5a2 --- /dev/null +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/internal/network/extensions/ChatMetadataExtensionTest.kt @@ -0,0 +1,60 @@ +package com.flipcash.services.internal.network.extensions + +import com.codeinc.flipcash.gen.chat.v1.Model as ChatModel +import com.codeinc.flipcash.gen.common.v1.Common +import com.codeinc.flipcash.gen.profile.v1.Model as ProfileModel +import com.google.protobuf.ByteString +import com.google.protobuf.Timestamp +import org.junit.Test +import kotlin.test.assertEquals + +/** + * The member id is what authorizes re-minting a profile picture's expired download URL, and the + * server sets it on the chat member rather than inside the nested profile. + */ +class ChatMetadataExtensionTest { + + private fun userId(byte: Byte): Common.UserId = + Common.UserId.newBuilder() + .setValue(ByteString.copyFrom(ByteArray(16) { byte })) + .build() + + private fun metadata(member: ChatModel.Member): ChatModel.Metadata = + ChatModel.Metadata.newBuilder() + .setChatId( + Common.ChatId.newBuilder() + .setValue(ByteString.copyFrom(ByteArray(32) { 1 })) + ) + .setType(ChatModel.ChatType.CONTACT_DM) + .setLastActivity(Timestamp.newBuilder().setSeconds(2000)) + .addMembers(member) + .build() + + @Test + fun `member profile takes the member's user id when the nested profile omits it`() { + val member = ChatModel.Member.newBuilder() + .setUserId(userId(9)) + .setUserProfile(ProfileModel.UserProfile.newBuilder().setDisplayName("User")) + .build() + + val result = metadata(member).toChatMetadata() + + assertEquals(ByteArray(16) { 9 }.toList(), result.members[0].userProfile.userId) + } + + @Test + fun `member profile keeps its own user id when the server sets one`() { + val member = ChatModel.Member.newBuilder() + .setUserId(userId(9)) + .setUserProfile( + ProfileModel.UserProfile.newBuilder() + .setDisplayName("User") + .setUserId(userId(3)) + ) + .build() + + val result = metadata(member).toChatMetadata() + + assertEquals(ByteArray(16) { 3 }.toList(), result.members[0].userProfile.userId) + } +} diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/BlobExpiryTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/BlobExpiryTest.kt new file mode 100644 index 0000000000..f034dc59c0 --- /dev/null +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/BlobExpiryTest.kt @@ -0,0 +1,116 @@ +package com.flipcash.services.models.chat + +import com.getcode.utils.base58 +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertSame +import kotlin.test.assertTrue +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * `DownloadUrl.expires_at` is what tells a persisted [MediaItem] that its URL needs re-minting + * before it can be fetched again — these cover the model's half of that. + */ +class BlobExpiryTest { + + private val now = Instant.fromEpochMilliseconds(1_700_000_000_000) + + private fun blob(url: String, expiresAt: Instant?) = BlobMetadata( + mimeType = "image/png", + sizeBytes = 1, + downloadUrl = url, + image = ImageMetadata(width = 160, height = 160, blurhash = "abc"), + expiresAtMillis = expiresAt?.toEpochMilliseconds(), + ) + + private fun rendition(id: Byte, longestSide: Int, expiresAt: Instant?) = MediaItemRendition( + role = MediaItemRendition.Role.THUMBNAIL, + blobId = BlobId(byteArrayOf(id)), + blob = BlobMetadata( + mimeType = "image/png", + sizeBytes = 1, + downloadUrl = "https://cdn/$id", + image = ImageMetadata(width = longestSide, height = longestSide, blurhash = "abc"), + expiresAtMillis = expiresAt?.toEpochMilliseconds(), + ), + ) + + @Test + fun `metadata without an expiry is never reported expired`() { + assertFalse(blob("https://cdn/a", expiresAt = null).isDownloadUrlExpired(now)) + } + + @Test + fun `an expiry comfortably ahead of now is not expired`() { + assertFalse(blob("https://cdn/a", now + 5.minutes).isDownloadUrlExpired(now)) + } + + @Test + fun `an expiry in the past is expired`() { + assertTrue(blob("https://cdn/a", now - 1.seconds).isDownloadUrlExpired(now)) + } + + @Test + fun `an expiry inside the margin is expired, so a URL cannot die mid-flight`() { + val insideMargin = now + BlobMetadata.EXPIRY_MARGIN - 1.seconds + assertTrue(blob("https://cdn/a", insideMargin).isDownloadUrlExpired(now)) + + val outsideMargin = now + BlobMetadata.EXPIRY_MARGIN + 1.seconds + assertFalse(blob("https://cdn/a", outsideMargin).isDownloadUrlExpired(now)) + } + + @Test + fun `expiresAt round-trips the stored millis`() { + assertEquals(now, blob("https://cdn/a", now).expiresAt) + assertNull(blob("https://cdn/a", null).expiresAt) + } + + @Test + fun `cacheKey is the base58 blob id, so it survives a re-mint`() { + val id = BlobId(byteArrayOf(7, 8, 9)) + val before = MediaItemRendition(MediaItemRendition.Role.THUMBNAIL, id, blob("https://cdn/a", now)) + val after = before.copy(blob = blob("https://cdn/b", now + 1.minutes)) + assertEquals(id.bytes.base58, before.cacheKey) + assertEquals(before.cacheKey, after.cacheKey) + } + + @Test + fun `expiredRenditionForSize only reports the rendition the size actually resolves to`() { + // The 160 the small surfaces use is still good; the 320 the large ones use has expired. + val item = MediaItem( + listOf( + rendition(id = 1, longestSide = 160, expiresAt = now + 5.minutes), + rendition(id = 2, longestSide = 320, expiresAt = now - 5.minutes), + ) + ) + assertNull(item.expiredRenditionForSize(96, now)) + assertEquals(byteArrayOf(2).base58, item.expiredRenditionForSize(300, now)?.cacheKey) + } + + @Test + fun `withRefreshedBlobs swaps only the renditions it was given`() { + val item = MediaItem( + listOf( + rendition(id = 1, longestSide = 160, expiresAt = now - 5.minutes), + rendition(id = 2, longestSide = 320, expiresAt = now - 5.minutes), + ) + ) + val fresh = blob("https://cdn/2-fresh", now + 10.minutes) + + val refreshed = item.withRefreshedBlobs(mapOf(byteArrayOf(2).base58 to fresh)) + + assertEquals("https://cdn/1", refreshed.urlForSize(96)) + assertEquals("https://cdn/2-fresh", refreshed.urlForSize(300)) + assertFalse(refreshed.renditionForSize(300)!!.isDownloadUrlExpired(now)) + } + + @Test + fun `withRefreshedBlobs on an empty map is the same instance`() { + val item = MediaItem(listOf(rendition(id = 1, longestSide = 160, expiresAt = null))) + assertSame(item, item.withRefreshedBlobs(emptyMap())) + } +}