From 61d7aae7ebad3124716560ac497c06bc59e64b4d Mon Sep 17 00:00:00 2001 From: sevenhill Date: Mon, 6 Jul 2026 09:10:31 +0300 Subject: [PATCH] Fix media placeholders and image cache --- .../main/java/xyz/kusoft/qmax/MainActivity.kt | 69 ++++++++++++-- .../xyz/kusoft/qmax/core/QMaxRepository.kt | 32 ++++++- .../qmax/core/local/AttachmentDiskCache.kt | 30 ++++++ .../qmax/core/local/LocalMessageCache.kt | 2 + .../xyz/kusoft/qmax/core/network/QMaxApi.kt | 4 +- .../java/xyz/kusoft/qmax/ui/QMaxViewModel.kt | 44 ++++++++- .../QMax.Api/Controllers/ChatsController.cs | 14 +++ .../QMax.Api/Services/MaxBridgeSyncService.cs | 2 +- .../QMax.Api/Services/MessageTextSanitizer.cs | 2 + tests/QMax.Tests/ApiSmokeTests.cs | 92 +++++++++++++++++++ 10 files changed, 275 insertions(+), 16 deletions(-) diff --git a/android/app/src/main/java/xyz/kusoft/qmax/MainActivity.kt b/android/app/src/main/java/xyz/kusoft/qmax/MainActivity.kt index 74cba42..fa91372 100644 --- a/android/app/src/main/java/xyz/kusoft/qmax/MainActivity.kt +++ b/android/app/src/main/java/xyz/kusoft/qmax/MainActivity.kt @@ -1310,6 +1310,8 @@ private fun cleanChatPreview(value: String?): String? { private fun isGenericMediaPreview(value: String?): Boolean { return when (value?.trim()?.lowercase()) { + "\u043c\u0435\u0434\u0438\u0430", + "media", "\u0444\u043e\u0442\u043e", "photo", "image", @@ -2600,14 +2602,27 @@ private fun MediaAlbumCell( } val model = cachedImagePath?.let(::File) ?: imageRequest(url, session.accessToken) val previewSource = cachedImagePath ?: url + var imageLoaded by remember(attachment.id, cachedImagePath, url) { mutableStateOf(false) } + var imageFailed by remember(attachment.id, cachedImagePath, url) { mutableStateOf(false) } AsyncImage( model = model, contentDescription = attachment.fileName, contentScale = ContentScale.Crop, modifier = Modifier .fillMaxSize() - .clickable { vm.openImage(previewSource, imagePreviewSources) } + .clickable { vm.openImage(previewSource, imagePreviewSources) }, + onSuccess = { + imageLoaded = true + imageFailed = false + }, + onError = { + imageLoaded = false + imageFailed = true + } ) + if (!imageLoaded) { + MediaImagePlaceholder(failed = imageFailed) + } } else { Box( modifier = Modifier @@ -2711,16 +2726,58 @@ private fun ImageAttachment( } val model = cachedImagePath?.let(::File) ?: imageRequest(url, token) val previewSource = cachedImagePath ?: url - AsyncImage( - model = model, - contentDescription = attachment.fileName, - contentScale = ContentScale.Crop, + var imageLoaded by remember(attachment.id, cachedImagePath, url) { mutableStateOf(false) } + var imageFailed by remember(attachment.id, cachedImagePath, url) { mutableStateOf(false) } + Box( modifier = Modifier .fillMaxWidth() .aspectRatio(1.25f) .background(Color(0xFFE1E8ED)) .clickable { vm.openImage(previewSource, imagePreviewSources) } - ) + ) { + AsyncImage( + model = model, + contentDescription = attachment.fileName, + contentScale = ContentScale.Crop, + modifier = Modifier.fillMaxSize(), + onSuccess = { + imageLoaded = true + imageFailed = false + }, + onError = { + imageLoaded = false + imageFailed = true + } + ) + if (!imageLoaded) { + MediaImagePlaceholder(failed = imageFailed) + } + } +} + +@Composable +private fun MediaImagePlaceholder(failed: Boolean) { + Box( + modifier = Modifier + .fillMaxSize() + .background(Color(0xFFE1E8ED)), + contentAlignment = Alignment.Center + ) { + if (failed) { + Icon( + Icons.AutoMirrored.Filled.InsertDriveFile, + contentDescription = null, + tint = QMaxMuted, + modifier = Modifier.size(28.dp) + ) + } else { + CircularProgressIndicator( + color = QMaxBlue, + strokeWidth = 2.dp, + modifier = Modifier.size(24.dp) + ) + } + } } @Composable diff --git a/android/app/src/main/java/xyz/kusoft/qmax/core/QMaxRepository.kt b/android/app/src/main/java/xyz/kusoft/qmax/core/QMaxRepository.kt index da8bcd7..11452ed 100644 --- a/android/app/src/main/java/xyz/kusoft/qmax/core/QMaxRepository.kt +++ b/android/app/src/main/java/xyz/kusoft/qmax/core/QMaxRepository.kt @@ -102,10 +102,10 @@ class QMaxRepository( return messageCache.messages(session, chatId) } - suspend fun messages(session: QMaxSession, chatId: String): List { - val messages = withFreshSession(session) { api.messages(it.serverUrl, it.accessToken, chatId) } + suspend fun messages(session: QMaxSession, chatId: String, sync: Boolean = false): List { + val messages = withFreshSession(session) { api.messages(it.serverUrl, it.accessToken, chatId, sync) } messageCache.saveMessages(session, chatId, messages) - return messages + return messageCache.messages(session, chatId) } suspend fun markChatRead(session: QMaxSession, chatId: String) { @@ -219,7 +219,7 @@ class QMaxRepository( suspend fun upload(session: QMaxSession, chatId: String, uri: Uri, caption: String?, replyToMessageId: String?): MessageDto { val meta = queryMeta(uri) val body = contentUriRequestBody(uri, meta.contentType) - return withFreshSession(session) { + val message = withFreshSession(session) { api.uploadAttachment( it.serverUrl, it.accessToken, @@ -230,7 +230,12 @@ class QMaxRepository( caption, replyToMessageId ) - }.also { messageCache.upsertMessage(session, it) } + } + messageCache.upsertMessage(session, message) + message.attachments.firstOrNull()?.let { attachment -> + cacheUploadedAttachment(session, attachment, uri) + } + return message } suspend fun uploadVoice(session: QMaxSession, chatId: String, file: File, replyToMessageId: String?): MessageDto { @@ -526,6 +531,22 @@ class QMaxRepository( } } + private suspend fun cacheUploadedAttachment(session: QMaxSession, attachment: AttachmentDto, uri: Uri) { + runCatching { + attachmentCache.storeFile(session, attachment) { target -> + context.contentResolver.openInputStream(uri).use { input -> + requireNotNull(input) { "Cannot open selected file" } + target.outputStream().use { output -> + input.copyTo(output) + } + } + target.length() + } + }.onFailure { error -> + Log.w(RepositoryLogTag, "Cannot prefill attachment cache", error) + } + } + private data class FileMeta(val fileName: String, val contentType: String) private suspend fun withFreshSession( @@ -573,6 +594,7 @@ class QMaxRepository( } } +private const val RepositoryLogTag = "QMaxRepository" private const val PushLogTag = "QMaxPush" private data class SharedAttachment( diff --git a/android/app/src/main/java/xyz/kusoft/qmax/core/local/AttachmentDiskCache.kt b/android/app/src/main/java/xyz/kusoft/qmax/core/local/AttachmentDiskCache.kt index 28da2ca..ff2751f 100644 --- a/android/app/src/main/java/xyz/kusoft/qmax/core/local/AttachmentDiskCache.kt +++ b/android/app/src/main/java/xyz/kusoft/qmax/core/local/AttachmentDiskCache.kt @@ -37,6 +37,36 @@ class AttachmentDiskCache(context: Context) { } } + suspend fun storeFile( + session: QMaxSession, + attachment: AttachmentDto, + writer: suspend (File) -> Long + ): File { + return withContext(Dispatchers.IO) { + val target = targetFile(session, attachment) + target.parentFile?.mkdirs() + val part = File(target.parentFile, target.name + ".part") + if (part.exists()) part.delete() + + writer(part) + + if (!isValid(part, attachment)) { + val expected = attachment.fileSizeBytes.takeIf { it > 0 }?.toString() ?: "non-empty" + val actual = if (part.exists()) part.length().toString() else "missing" + part.delete() + error("Attachment cache mismatch: expected $expected, got $actual") + } + + if (target.exists()) target.delete() + if (!part.renameTo(target)) { + part.delete() + error("Cannot move uploaded file into attachment cache") + } + + target + } + } + suspend fun clearAll() { withContext(Dispatchers.IO) { root.deleteRecursively() diff --git a/android/app/src/main/java/xyz/kusoft/qmax/core/local/LocalMessageCache.kt b/android/app/src/main/java/xyz/kusoft/qmax/core/local/LocalMessageCache.kt index a422f40..2e2cca5 100644 --- a/android/app/src/main/java/xyz/kusoft/qmax/core/local/LocalMessageCache.kt +++ b/android/app/src/main/java/xyz/kusoft/qmax/core/local/LocalMessageCache.kt @@ -145,6 +145,8 @@ class LocalMessageCache(context: Context) { private fun isGenericMediaLabel(value: String?): Boolean { return when (value?.trim()?.lowercase()) { + "\u043c\u0435\u0434\u0438\u0430", + "media", "\u0444\u043e\u0442\u043e", "photo", "image", diff --git a/android/app/src/main/java/xyz/kusoft/qmax/core/network/QMaxApi.kt b/android/app/src/main/java/xyz/kusoft/qmax/core/network/QMaxApi.kt index 085a611..a97334a 100644 --- a/android/app/src/main/java/xyz/kusoft/qmax/core/network/QMaxApi.kt +++ b/android/app/src/main/java/xyz/kusoft/qmax/core/network/QMaxApi.kt @@ -79,8 +79,8 @@ class QMaxApi { return post(sessionServerUrl, "/api/chats/direct", token, CreateDirectChatRequest(externalChatId, title)) } - suspend fun messages(sessionServerUrl: String, token: String, chatId: String): List { - return get(sessionServerUrl, "/api/chats/$chatId/messages?sync=false", token) + suspend fun messages(sessionServerUrl: String, token: String, chatId: String, sync: Boolean = false): List { + return get(sessionServerUrl, "/api/chats/$chatId/messages?sync=$sync", token) } suspend fun markChatRead(sessionServerUrl: String, token: String, chatId: String) { diff --git a/android/app/src/main/java/xyz/kusoft/qmax/ui/QMaxViewModel.kt b/android/app/src/main/java/xyz/kusoft/qmax/ui/QMaxViewModel.kt index 31ff8cd..0a8fb6c 100644 --- a/android/app/src/main/java/xyz/kusoft/qmax/ui/QMaxViewModel.kt +++ b/android/app/src/main/java/xyz/kusoft/qmax/ui/QMaxViewModel.kt @@ -81,6 +81,8 @@ class QMaxViewModel(private val repository: QMaxRepository) : ViewModel() { private val locallyReadChatFingerprints = mutableMapOf() private val imageCacheInFlight = mutableSetOf() private val avatarCacheInFlight = mutableSetOf() + private val messageHydrationInFlight = mutableSetOf() + private val lastMessageHydrationAt = mutableMapOf() init { viewModelScope.launch { @@ -88,6 +90,10 @@ class QMaxViewModel(private val repository: QMaxRepository) : ViewModel() { val previousSession = state.value.session val sessionChanged = previousSession?.serverUrl != session?.serverUrl || previousSession?.userName != session?.userName + if (sessionChanged) { + messageHydrationInFlight.clear() + lastMessageHydrationAt.clear() + } state.value = state.value.copy( session = session, serverUrl = session?.serverUrl ?: state.value.serverUrl, @@ -330,7 +336,7 @@ class QMaxViewModel(private val repository: QMaxRepository) : ViewModel() { } realtime.joinChat(chat.id) } - loadMessages(showLoading = false) + loadMessages(showLoading = false, forceHydrate = true) presencePollJob?.cancel() presencePollJob = viewModelScope.launch { while (state.value.selectedChat?.id == chat.id) { @@ -456,7 +462,7 @@ class QMaxViewModel(private val repository: QMaxRepository) : ViewModel() { refreshChats(showLoading = false) } - fun loadMessages(showLoading: Boolean = true) = launchLoading(showLoading) { + fun loadMessages(showLoading: Boolean = true, forceHydrate: Boolean = false) = launchLoading(showLoading) { if (!showLoading && state.value.sendingMessage) { return@launchLoading } @@ -485,6 +491,39 @@ class QMaxViewModel(private val repository: QMaxRepository) : ViewModel() { messages = fresh ) persistCurrentChats() + hydrateMessages(chat.id, force = forceHydrate) + } + } + + private fun hydrateMessages(chatId: String, force: Boolean = false) { + val session = state.value.session ?: return + val now = System.currentTimeMillis() + val lastHydratedAt = lastMessageHydrationAt[chatId] ?: 0L + if (!force && now - lastHydratedAt < MessageHydrationIntervalMs) { + return + } + if (!messageHydrationInFlight.add(chatId)) { + return + } + + lastMessageHydrationAt[chatId] = now + viewModelScope.launch { + try { + val hydrated = withTimeout(MessageSyncTimeoutMs) { + repository.messages(session, chatId, sync = true) + } + if (state.value.selectedChat?.id == chatId) { + state.value = state.value.copy(messages = hydrated) + } + } catch (error: CancellationException) { + throw error + } catch (error: Throwable) { + if (BuildConfig.DEBUG) { + Log.d(LogTag, "Message hydration failed", error) + } + } finally { + messageHydrationInFlight.remove(chatId) + } } } @@ -1236,6 +1275,7 @@ class QMaxViewModel(private val repository: QMaxRepository) : ViewModel() { const val LogTag = "QMAX" const val ChatListTimeoutMs = 45_000L const val MessageSyncTimeoutMs = 120_000L + const val MessageHydrationIntervalMs = 120_000L const val AutoLoginDelayMs = 1_000L const val PushRegistrationAttempts = 6 const val InitialPushRegistrationRetryMs = 15_000L diff --git a/server/QMax.Api/Controllers/ChatsController.cs b/server/QMax.Api/Controllers/ChatsController.cs index 06950aa..11fb0d3 100644 --- a/server/QMax.Api/Controllers/ChatsController.cs +++ b/server/QMax.Api/Controllers/ChatsController.cs @@ -106,6 +106,7 @@ public sealed class ChatsController( } var messages = (await query.ToArrayAsync(cancellationToken)) + .Where(message => !IsGenericPreviewPlaceholder(message)) .OrderByDescending(x => x.SentAt) .Take(Math.Clamp(take, 1, 200)) .ToArray(); @@ -121,6 +122,11 @@ public sealed class ChatsController( return; } + if (MessageTextSanitizer.IsGenericMediaLabel(preview)) + { + return; + } + var previewAt = chat.LastMessageAt.Value; if (chat.HistoryClearedAt is { } historyClearedAt && previewAt <= historyClearedAt) { @@ -202,6 +208,14 @@ public sealed class ChatsController( return hash.ToString("x8"); } + private static bool IsGenericPreviewPlaceholder(Message message) + { + return message.Direction == MessageDirection.Incoming && + message.Attachments.Count == 0 && + message.ExternalId?.StartsWith("preview:", StringComparison.Ordinal) == true && + MessageTextSanitizer.IsGenericMediaLabel(message.Text); + } + [HttpPost("{chatId:guid}/read")] public async Task MarkRead(Guid chatId, CancellationToken cancellationToken) { diff --git a/server/QMax.Api/Services/MaxBridgeSyncService.cs b/server/QMax.Api/Services/MaxBridgeSyncService.cs index 18ff34e..f9ced5d 100644 --- a/server/QMax.Api/Services/MaxBridgeSyncService.cs +++ b/server/QMax.Api/Services/MaxBridgeSyncService.cs @@ -422,7 +422,7 @@ public sealed class MaxBridgeSyncService( chat.UnreadCount++; } - var message = shouldNotify + var message = shouldNotify && !MessageTextSanitizer.IsGenericMediaLabel(preview) ? CreatePreviewMessage(chat, preview, previewAt) : null; return new ListPreviewApplyResult(true, message); diff --git a/server/QMax.Api/Services/MessageTextSanitizer.cs b/server/QMax.Api/Services/MessageTextSanitizer.cs index d1f43a9..ad8624e 100644 --- a/server/QMax.Api/Services/MessageTextSanitizer.cs +++ b/server/QMax.Api/Services/MessageTextSanitizer.cs @@ -35,6 +35,8 @@ public static class MessageTextSanitizer { return value?.Trim().ToLowerInvariant() switch { + "\u043c\u0435\u0434\u0438\u0430" => true, + "media" => true, "\u0444\u043e\u0442\u043e" => true, "photo" => true, "image" => true, diff --git a/tests/QMax.Tests/ApiSmokeTests.cs b/tests/QMax.Tests/ApiSmokeTests.cs index c2e4233..f732141 100644 --- a/tests/QMax.Tests/ApiSmokeTests.cs +++ b/tests/QMax.Tests/ApiSmokeTests.cs @@ -773,6 +773,40 @@ public sealed class ApiSmokeTests : IDisposable Assert.StartsWith("preview:", backfilled.ExternalId); } + [Fact] + public async Task GetMessagesDoesNotBackfillGenericMediaPreviewWhenAttachmentRowIsMissing() + { + using var factory = _factory.WithWebHostBuilder(builder => + { + builder.ConfigureTestServices(services => + { + services.RemoveAll(); + }); + }); + using var client = factory.CreateClient(); + await LoginAsync(client); + + var marker = Guid.NewGuid().ToString("N"); + var chat = await CreateDirectChatAsync(client, $"generic-media-preview-chat-{marker}", "Generic Media Preview"); + await using (var scope = factory.Services.CreateAsyncScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var stored = await db.Chats.SingleAsync(x => x.Id == chat.Id); + stored.LastMessagePreview = "\u0424\u043e\u0442\u043e"; + stored.LastMessageAt = DateTimeOffset.UtcNow; + stored.UnreadCount = 1; + await db.SaveChangesAsync(); + } + + var messages = await client.GetFromJsonAsync($"/api/chats/{chat.Id}/messages", JsonOptions); + + Assert.NotNull(messages); + Assert.DoesNotContain(messages!, x => x.ExternalId?.StartsWith("preview:", StringComparison.Ordinal) == true); + await using var verifyScope = factory.Services.CreateAsyncScope(); + var verifyDb = verifyScope.ServiceProvider.GetRequiredService(); + Assert.False(await verifyDb.Messages.AnyAsync(x => x.ChatId == chat.Id)); + } + [Fact] public async Task SyncCreatesMessageRowForFreshPreviewOnlyIncomingUpdate() { @@ -836,6 +870,64 @@ public sealed class ApiSmokeTests : IDisposable } } + [Fact] + public async Task SyncDoesNotCreateTextOnlyMessageForGenericMediaPreviewOnlyIncomingUpdate() + { + var marker = Guid.NewGuid().ToString("N"); + var externalId = $"generic-media-sync-chat-{marker}"; + var now = DateTimeOffset.UtcNow; + var bridge = new ResolvingDeleteMaxBridgeClient + { + Updates = + [ + new MaxChatUpdate( + externalId, + "Generic Media Sync", + null, + now, + [], + "\u0424\u043e\u0442\u043e", + false, + now, + now.ToOffset(TimeSpan.FromHours(3)).ToString("HH:mm"), + $"https://max.test/chat/{externalId}") + ] + }; + using var factory = _factory.WithWebHostBuilder(builder => + { + builder.ConfigureTestServices(services => + { + services.RemoveAll(); + services.RemoveAll(); + services.AddSingleton(bridge); + }); + }); + using var client = factory.CreateClient(); + await LoginAsync(client); + + var chat = await CreateDirectChatAsync(client, externalId, "Generic Media Sync"); + await using (var scope = factory.Services.CreateAsyncScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var stored = await db.Chats.SingleAsync(x => x.Id == chat.Id); + stored.LastMessagePreview = "previous preview"; + stored.LastMessageAt = now.AddMinutes(-5); + await db.SaveChangesAsync(); + } + + var sync = await client.PostAsync("/api/max/sync", null); + await AssertStatusAsync(HttpStatusCode.OK, sync); + + await using var verifyScope = factory.Services.CreateAsyncScope(); + var verifyDb = verifyScope.ServiceProvider.GetRequiredService(); + var storedChat = await verifyDb.Chats.SingleAsync(x => x.Id == chat.Id); + Assert.Equal("\u041c\u0435\u0434\u0438\u0430", storedChat.LastMessagePreview); + Assert.False(await verifyDb.Messages.AnyAsync(x => + x.ChatId == chat.Id && + x.ExternalId != null && + x.ExternalId.StartsWith("preview:"))); + } + [Fact] public async Task BulkDeleteOutboxRefreshesStaleChatUrlAfterMenuOpenFailure() {