Skip to content

Commit e3ba086

Browse files
committed
refactor(chat): extract loadInitialMessages branches into named functions
Turn the two-way choice in loadInitialMessages into a plain if/else on a single named condition, weLikelyOnlyHaveASmallBacklog, and extract each branch body into its own function: closeBacklogFromNewestOfflineMessage and fetchNewestMessagesForInitialLoad. No behavior change. closeBacklogFromNewestOfflineMessage's fallback-safety rationale and fetchNewestMessagesForInitialLoad's per-case log messages are now documented against the actual booleans they depend on, rather than being inferred from which when-branch happened to call them. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
1 parent 519c08e commit e3ba086

1 file changed

Lines changed: 53 additions & 50 deletions

File tree

app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt

Lines changed: 53 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@ class OfflineFirstChatRepository @Inject constructor(
173173
}
174174
}
175175

176-
@Suppress("LongMethod")
177176
override suspend fun loadInitialMessages(withNetworkParams: Bundle) {
178177
logger.d(TAG, "---- loadInitialMessages ------------")
179178
cleanupExpiredMessages()
@@ -186,64 +185,68 @@ class OfflineFirstChatRepository @Inject constructor(
186185
Log.d(TAG, "newestMessageIdFromDb: $newestMessageIdFromDb")
187186

188187
val weAlreadyHaveSomeOfflineMessages = newestMessageIdFromDb > 0
189-
190188
val weHaveAtLeastTheLastReadMessage = newestMessageIdFromDb >= conversationModel.lastReadMessage.toLong()
189+
val weLikelyOnlyHaveASmallBacklog = weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage
190+
191191
Log.d(TAG, "weAlreadyHaveSomeOfflineMessages:$weAlreadyHaveSomeOfflineMessages")
192192
Log.d(TAG, "weHaveAtLeastTheLastReadMessage:$weHaveAtLeastTheLastReadMessage")
193+
Log.d(TAG, "weLikelyOnlyHaveASmallBacklog:$weLikelyOnlyHaveASmallBacklog")
193194

194-
when {
195-
weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage -> {
196-
// Close the backlog since the newest offline message. This is required on
197-
// chat-relay servers (the relay cannot deliver messages that arrived while the
198-
// app was closed) and is equally cheap on long-polling servers, where it just
199-
// front-loads what the first poll request would have fetched. This way the
200-
// initial load never has to know the live-update mode, i.e. it must not wait for
201-
// the websocket. closeBacklog loops until the backlog is fully closed — a single
202-
// capped fetch could leave a permanent gap on chat-relay servers.
203-
//
204-
// This request is always made, even if the conversation's cached lastMessage
205-
// suggests we are already caught up: that value is only as fresh as the last room
206-
// list sync and can already be behind the server by the time the chat is opened.
207-
// closeBacklog is cheap when there is truly nothing new (a single request coming
208-
// back empty), so there is no good reason to trust the stale value instead.
209-
Log.d(TAG, "Closing the backlog from the newest offline message for initial loading")
210-
211-
syncer.closeBacklog(
212-
target = syncTarget,
213-
fromMessageId = newestMessageIdFromDb,
214-
lastCommonRead = newXChatLastCommonRead,
215-
events = syncEvents
216-
)
217-
}
195+
if (weLikelyOnlyHaveASmallBacklog) {
196+
closeBacklogFromNewestOfflineMessage(newestMessageIdFromDb)
197+
} else {
198+
fetchNewestMessagesForInitialLoad(
199+
withNetworkParams,
200+
weAlreadyHaveSomeOfflineMessages,
201+
weHaveAtLeastTheLastReadMessage
202+
)
203+
}
204+
}
218205

219-
else -> {
220-
if (!weAlreadyHaveSomeOfflineMessages) {
221-
Log.d(TAG, "An online request for newest 100 messages is made because offline chat is empty")
222-
if (networkMonitor.isOnline.value.not()) {
223-
// _generalUIFlow.emit(ChatActivity.NO_OFFLINE_MESSAGES_FOUND)
224-
}
225-
} else {
226-
Log.d(
227-
TAG,
228-
"An online request for newest 100 messages is made because we don't have the " +
229-
"lastReadMessage (gaps could be closed by scrolling up to merge the chatblocks)"
230-
)
231-
}
206+
/**
207+
* Tries to close the backlog since the newest offline message.
208+
*/
209+
private suspend fun closeBacklogFromNewestOfflineMessage(newestMessageIdFromDb: Long) {
210+
Log.d(TAG, "Closing the backlog from the newest offline message for initial loading")
232211

233-
// set up field map to load the newest messages
234-
val fieldMap = getFieldMap(
235-
lookIntoFuture = false,
236-
timeout = 0,
237-
includeLastKnown = true,
238-
lastKnown = null
239-
)
240-
withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap)
241-
withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token)
212+
syncer.closeBacklog(
213+
target = syncTarget,
214+
fromMessageId = newestMessageIdFromDb,
215+
lastCommonRead = newXChatLastCommonRead,
216+
events = syncEvents
217+
)
218+
}
242219

243-
Log.d(TAG, "Starting online request for initial loading")
244-
getAndPersistMessages(withNetworkParams)
220+
private suspend fun fetchNewestMessagesForInitialLoad(
221+
withNetworkParams: Bundle,
222+
weAlreadyHaveSomeOfflineMessages: Boolean,
223+
weHaveAtLeastTheLastReadMessage: Boolean
224+
) {
225+
if (!weAlreadyHaveSomeOfflineMessages) {
226+
Log.d(TAG, "An online request for newest 100 messages is made because offline chat is empty")
227+
if (networkMonitor.isOnline.value.not()) {
228+
// _generalUIFlow.emit(ChatActivity.NO_OFFLINE_MESSAGES_FOUND)
245229
}
230+
} else if (!weHaveAtLeastTheLastReadMessage) {
231+
Log.d(
232+
TAG,
233+
"An online request for newest 100 messages is made because we don't have the " +
234+
"lastReadMessage (gaps could be closed by scrolling up to merge the chatblocks)"
235+
)
246236
}
237+
238+
// set up field map to load the newest messages
239+
val fieldMap = getFieldMap(
240+
lookIntoFuture = false,
241+
timeout = 0,
242+
includeLastKnown = true,
243+
lastKnown = null
244+
)
245+
withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap)
246+
withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token)
247+
248+
Log.d(TAG, "Starting online request for initial loading")
249+
getAndPersistMessages(withNetworkParams)
247250
}
248251

249252
override suspend fun startMessagePolling(hasHighPerformanceBackend: Boolean) {

0 commit comments

Comments
 (0)