From 6ed8e88bf0a1b5299668bfb47215b48f4349bb28 Mon Sep 17 00:00:00 2001 From: James Magahern Date: Wed, 10 Apr 2024 23:35:54 -0700 Subject: [PATCH] More pleasant OOTB experience --- .../ConversationListScreen.kt | 102 ++++++++++++------ .../ConversationListViewModel.kt | 22 ++-- .../ui/conversationlist/NoContentView.kt | 2 +- app/src/main/res/drawable/error.xml | 10 ++ .../kordophone/backend/server/APIClient.kt | 41 ++++--- .../backend/server/ChatRepository.kt | 44 +++++--- .../backend/server/UpdateMonitor.kt | 5 + 7 files changed, 159 insertions(+), 67 deletions(-) create mode 100644 app/src/main/res/drawable/error.xml diff --git a/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListScreen.kt b/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListScreen.kt index 3cc3ed2..82ca221 100644 --- a/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListScreen.kt +++ b/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListScreen.kt @@ -2,12 +2,29 @@ package net.buzzert.kordophonedroid.ui.conversationlist import androidx.compose.foundation.background import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.* +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape -import androidx.compose.material.* +import androidx.compose.material.Divider +import androidx.compose.material.ExperimentalMaterialApi +import androidx.compose.material.Icon +import androidx.compose.material.IconButton +import androidx.compose.material.MaterialTheme +import androidx.compose.material.Scaffold +import androidx.compose.material.Text +import androidx.compose.material.TopAppBar import androidx.compose.material.icons.Icons import androidx.compose.material.icons.rounded.Settings import androidx.compose.material.pullrefresh.PullRefreshIndicator @@ -27,10 +44,11 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp -import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.compose.collectAsStateWithLifecycle import kotlinx.coroutines.launch import net.buzzert.kordophone.backend.model.Conversation +import net.buzzert.kordophonedroid.R import net.buzzert.kordophonedroid.ui.Destination import net.buzzert.kordophonedroid.ui.LocalNavController import java.time.LocalDate @@ -54,8 +72,12 @@ fun ConversationListScreen( viewModel: ConversationListViewModel = hiltViewModel(), ) { val conversations by viewModel.conversations.collectAsStateWithLifecycle(initialValue = emptyList()) + val encounteredError by viewModel.encounteredConnectionError + ConversationListView( conversations = conversations, + isConfigured = viewModel.isServerConfigured, + encounteredError = encounteredError, onRefresh = suspend { viewModel.refresh() } ) } @@ -64,6 +86,8 @@ fun ConversationListScreen( @OptIn(ExperimentalMaterialApi::class) fun ConversationListView( conversations: List, + isConfigured: Boolean = true, + encounteredError: Boolean = false, onRefresh: suspend () -> Unit, ) { val listState = rememberLazyListState() @@ -77,6 +101,8 @@ fun ConversationListView( refreshing = false } + val showErrorScreen = conversations.isEmpty() && encounteredError + val refreshState = rememberPullRefreshState(refreshing = refreshing, onRefresh = ::refresh) val navController = LocalNavController.current @@ -90,35 +116,49 @@ fun ConversationListView( }) } ) { - Box(Modifier.pullRefresh(refreshState)) { - LazyColumn( - state = listState, - modifier = Modifier - .padding(it) - .fillMaxSize() - ) { - items(conversations) { conversation -> - val clickHandler = { - val route = Destination.MessageList.createRoute(conversation.guid) - navController.navigate(route) - } - - ConversationListItem( - name = conversation.formattedDisplayName(), - id = conversation.guid, - isUnread = conversation.unreadCount > 0, - lastMessagePreview = conversation.lastMessagePreview ?: "", - date = conversation.date, - onClick = clickHandler - ) - } - } - - PullRefreshIndicator( - refreshing = refreshing, - state = refreshState, - modifier = Modifier.align(Alignment.TopCenter), + if (showErrorScreen) { + NoContentView( + icon = R.drawable.error, + text = "Connection error", + onSettings = onSettingsInvoked ) + } else if (!isConfigured) { + NoContentView( + icon = R.drawable.storage, + text = "Server not configured", + onSettings = onSettingsInvoked + ) + } else { + Box(Modifier.pullRefresh(refreshState)) { + LazyColumn( + state = listState, + modifier = Modifier + .padding(it) + .fillMaxSize() + ) { + items(conversations) { conversation -> + val clickHandler = { + val route = Destination.MessageList.createRoute(conversation.guid) + navController.navigate(route) + } + + ConversationListItem( + name = conversation.formattedDisplayName(), + id = conversation.guid, + isUnread = conversation.unreadCount > 0, + lastMessagePreview = conversation.lastMessagePreview ?: "", + date = conversation.date, + onClick = clickHandler + ) + } + } + + PullRefreshIndicator( + refreshing = refreshing, + state = refreshState, + modifier = Modifier.align(Alignment.TopCenter), + ) + } } } } diff --git a/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListViewModel.kt b/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListViewModel.kt index 3c0f3e6..235399c 100644 --- a/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListViewModel.kt +++ b/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/ConversationListViewModel.kt @@ -1,26 +1,22 @@ package net.buzzert.kordophonedroid.ui.conversationlist import android.util.Log +import androidx.compose.runtime.State +import androidx.compose.runtime.mutableStateOf import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.cache import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import net.buzzert.kordophone.backend.model.Conversation import net.buzzert.kordophone.backend.server.APIClientFactory -import net.buzzert.kordophone.backend.server.Authentication import net.buzzert.kordophone.backend.server.ChatRepository -import net.buzzert.kordophone.backend.server.RetrofitAPIClient import net.buzzert.kordophonedroid.ui.shared.ServerConfigRepository -import okhttp3.HttpUrl -import okhttp3.HttpUrl.Companion.toHttpUrlOrNull -import java.net.URL import javax.inject.Inject const val CL_VM_LOG: String = "ConversationListViewModel" @@ -38,6 +34,14 @@ class ConversationListViewModel @Inject constructor( .reversed() } + val isServerConfigured: Boolean + get() = chatRepository.isConfigured + + val encounteredConnectionError: State + get() = _encounteredConnectionError + + private val _encounteredConnectionError = mutableStateOf(false) + init { // Watch for config changes viewModelScope.launch { @@ -56,6 +60,12 @@ class ConversationListViewModel @Inject constructor( } } } + + viewModelScope.launch { + chatRepository.errorEncounteredChannel.collect { + _encounteredConnectionError.value = true + } + } } suspend fun refresh() { diff --git a/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/NoContentView.kt b/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/NoContentView.kt index 2f26fd4..6d8a229 100644 --- a/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/NoContentView.kt +++ b/app/src/main/java/net/buzzert/kordophonedroid/ui/conversationlist/NoContentView.kt @@ -24,7 +24,7 @@ fun NoContentView( @DrawableRes icon: Int, text: String, onSettings: () -> Unit, - modifier: Modifier, + modifier: Modifier = Modifier, ) { Column( modifier = modifier diff --git a/app/src/main/res/drawable/error.xml b/app/src/main/res/drawable/error.xml new file mode 100644 index 0000000..8355c11 --- /dev/null +++ b/app/src/main/res/drawable/error.xml @@ -0,0 +1,10 @@ + + + diff --git a/backend/src/main/java/net/buzzert/kordophone/backend/server/APIClient.kt b/backend/src/main/java/net/buzzert/kordophone/backend/server/APIClient.kt index d4ec9c8..d291493 100644 --- a/backend/src/main/java/net/buzzert/kordophone/backend/server/APIClient.kt +++ b/backend/src/main/java/net/buzzert/kordophone/backend/server/APIClient.kt @@ -20,6 +20,8 @@ import retrofit2.converter.gson.GsonConverterFactory import java.net.URL interface APIClient { + val isConfigured: Boolean + fun getAPIInterface(): APIInterface fun getWebSocketClient( serverPath: String, @@ -99,11 +101,12 @@ class APIClientFactory { companion object { fun createClient(serverString: String?, authentication: Authentication?): APIClient { if (serverString == null || authentication == null) { - return InvalidConfigurationAPIClient() + return InvalidConfigurationAPIClient(InvalidConfigurationAPIClient.Issue.NOT_CONFIGURED) } // Try to parse server string - val serverURL = HttpUrl.parse(serverString) ?: return InvalidConfigurationAPIClient() + val serverURL = HttpUrl.parse(serverString) + ?: return InvalidConfigurationAPIClient(InvalidConfigurationAPIClient.Issue.INVALID_HOST_URL) return RetrofitAPIClient(serverURL.url(), authentication) } @@ -111,10 +114,23 @@ class APIClientFactory { } // TODO: Is this a dumb idea? -class InvalidConfigurationAPIClient: APIClient { - private class InvalidConfigurationAPIInterface: APIInterface { +class InvalidConfigurationAPIClient(val issue: Issue): APIClient { + enum class Issue { + NOT_CONFIGURED, + INVALID_CONFIGURATION, + INVALID_HOST_URL, + } + + class NotConfiguredError: Throwable(message = "Not configured.") + class InvalidConfigurationError(submessage: String): Throwable(message = "Invalid configuration: $submessage") + + private class InvalidConfigurationAPIInterface(val issue: Issue): APIInterface { private fun throwError(): Nothing { - throw Error("Invalid Configuration.") + when (issue) { + Issue.NOT_CONFIGURED -> throw NotConfiguredError() + Issue.INVALID_CONFIGURATION -> throw InvalidConfigurationError("Unknown.") + Issue.INVALID_HOST_URL -> throw InvalidConfigurationError("Invalid host URL.") + } } override suspend fun getVersion(): ResponseBody = throwError() @@ -124,16 +140,14 @@ class InvalidConfigurationAPIClient: APIClient { override suspend fun fetchAttachment(guid: String, preview: Boolean): ResponseBody = throwError() override suspend fun uploadAttachment(filename: String, body: RequestBody): retrofit2.Response = throwError() override suspend fun authenticate(request: AuthenticationRequest): retrofit2.Response = throwError() - override suspend fun getMessages( - conversationGUID: String, - limit: Int?, - beforeMessageGUID: GUID?, - afterMessageGUID: GUID? - ): retrofit2.Response> = throwError() + override suspend fun getMessages(conversationGUID: String, limit: Int?, beforeMessageGUID: GUID?, afterMessageGUID: GUID?): retrofit2.Response> = throwError() } + override val isConfigured: Boolean + get() { return issue != Issue.NOT_CONFIGURED } + override fun getAPIInterface(): APIInterface { - return InvalidConfigurationAPIInterface() + return InvalidConfigurationAPIInterface(issue) } override fun getWebSocketClient( @@ -162,6 +176,9 @@ class RetrofitAPIClient( .addConverterFactory(GsonConverterFactory.create()) .build() + override val isConfigured: Boolean + get() = true + override fun getAPIInterface(): APIInterface { return retrofit.create(APIInterface::class.java) } diff --git a/backend/src/main/java/net/buzzert/kordophone/backend/server/ChatRepository.kt b/backend/src/main/java/net/buzzert/kordophone/backend/server/ChatRepository.kt index f07043e..6d06522 100644 --- a/backend/src/main/java/net/buzzert/kordophone/backend/server/ChatRepository.kt +++ b/backend/src/main/java/net/buzzert/kordophone/backend/server/ChatRepository.kt @@ -28,16 +28,12 @@ import java.io.InputStream import java.util.Date import java.util.UUID import java.util.concurrent.ArrayBlockingQueue -import kotlin.Boolean -import kotlin.Int -import kotlin.String -import kotlin.let const val REPO_LOG: String = "ChatRepository" const val CONVERSATION_MESSAGE_SYNC_COUNT = 10 class ChatRepository( - apiClient: APIClient, + private var apiClient: APIClient, private val database: CachedChatDatabase, ) { sealed class Error { @@ -48,6 +44,11 @@ class ChatRepository( override val title: String = "Connection Error" override val description: String = message ?: "???" } + + data class AttachmentError(val message: String): Error() { + override val title: String = "Attachment Error" + override val description: String = message + } } // All (Cached) Conversations @@ -71,6 +72,10 @@ class ChatRepository( val errorEncounteredChannel: SharedFlow get() = _errorEncounteredChannel.asSharedFlow() + val isConfigured: Boolean + get() = apiClient.isConfigured + + // New messages for a particular conversation fun messagesChanged(conversation: Conversation): Flow> = database.messagesChanged(conversation) @@ -99,6 +104,7 @@ class ChatRepository( private var updateWatchScope: CoroutineScope? = null fun updateAPIClient(client: APIClient) { + this.apiClient = client this.apiInterface = client.getAPIInterface() this.updateMonitor = UpdateMonitor(client) @@ -160,7 +166,7 @@ class ChatRepository( return database.fetchMessages(conversation) } - suspend fun synchronize() = try { + suspend fun synchronize() = withErrorChannelHandling { Log.d(REPO_LOG, "Synchronizing conversations") // Sync conversations @@ -173,26 +179,17 @@ class ChatRepository( for (conversation in sortedConversations.take(CONVERSATION_MESSAGE_SYNC_COUNT)) { synchronizeConversation(conversation) } - } catch (e: java.lang.Exception) { - _errorEncounteredChannel.emit(Error.ConnectionError(e.message)) - } catch (e: java.lang.Error) { - _errorEncounteredChannel.emit(Error.ConnectionError(e.message)) } - suspend fun synchronizeConversation(conversation: Conversation, limit: Int = 15) = try { + suspend fun synchronizeConversation(conversation: Conversation, limit: Int = 15) = withErrorChannelHandling { // TODO: Should only fetch messages after the last GUID we know about. // But keep in mind that outgoing message GUIDs are fake... val messages = fetchMessages(conversation, limit = limit) database.writeMessages(messages, conversation) - } catch (e: java.lang.Exception) { - _errorEncounteredChannel.emit(Error.ConnectionError(e.message)) } - suspend fun markConversationAsRead(conversation: Conversation) = try { + suspend fun markConversationAsRead(conversation: Conversation) = withErrorChannelHandling(silent = true) { apiInterface.markConversation(conversation.guid) - } catch (e: java.lang.Exception) { - // Don't report via the channel, but log it. - Log.e(REPO_LOG, "Error marking conversation as read: ${e.message}") } suspend fun fetchAttachmentDataSource(guid: String, preview: Boolean): BufferedSource { @@ -217,6 +214,18 @@ class ChatRepository( // - private + private suspend fun withErrorChannelHandling(silent: Boolean = false, body: suspend () -> Unit) { + try { + body() + } catch (e: InvalidConfigurationAPIClient.NotConfiguredError) { + // Not configured yet: ignore. + } catch (e: java.lang.Exception) { + if (!silent) _errorEncounteredChannel.emit(Error.ConnectionError(e.message)) + } catch (e: java.lang.Error) { + if (!silent) _errorEncounteredChannel.emit(Error.ConnectionError(e.message)) + } + } + private suspend fun fetchConversations(): List { return apiInterface.getConversations().bodyOnSuccessOrThrow() } @@ -279,6 +288,7 @@ class ChatRepository( } } catch (e: java.lang.Exception) { Log.e(REPO_LOG, "Error uploading attachment (${e.message}). Dropping...") + _errorEncounteredChannel.emit(Error.AttachmentError("Upload error: ${e.message}")) } try { diff --git a/backend/src/main/java/net/buzzert/kordophone/backend/server/UpdateMonitor.kt b/backend/src/main/java/net/buzzert/kordophone/backend/server/UpdateMonitor.kt index 220a042..5fb7527 100644 --- a/backend/src/main/java/net/buzzert/kordophone/backend/server/UpdateMonitor.kt +++ b/backend/src/main/java/net/buzzert/kordophone/backend/server/UpdateMonitor.kt @@ -40,6 +40,11 @@ class UpdateMonitor(private val client: APIClient) : WebSocketListener() { private val _messageAdded: MutableSharedFlow = MutableSharedFlow() fun beginMonitoringUpdates() { + if (!client.isConfigured) { + Log.e(UPMON_LOG, "Closing websocket connection because client is not configured.") + return + } + Log.d(UPMON_LOG, "Opening websocket connection") try { this.webSocket = client.getWebSocketClient(