Ask the server only for the conversations that changed by AndyScherzinger · Pull Request #6744 · nextcloud/talk-android

AndyScherzinger · 2026-09-21T14:12:46Z

Stack — review and merge bottom-up. Each PR targets the branch below it, so it shows its parents' commits until they merge.

#6744 — modifiedSince delta sync ← you are here

#6745 — periodic background refresh

#6746 — foreground refresh loop

#6763 — prefetch marking + mobile-preload-chat capability

Every conversation list sync is a full GET /room today: every room, with its last message. This adds the server's modifiedSince delta so a sync carries only what changed. It is the prerequisite for the two periodic triggers stacked on top of it, which would otherwise multiply the most expensive request the client makes.

What it does

Sends modifiedSince on GET /room and echoes back the X-Nextcloud-Talk-Modified-Before response header on the next request — the server's own timestamp, never a client clock.
Drops includeStatus on delta syncs, which otherwise makes the server return every one-to-one room regardless and eats most of the saving.
Still forces a full sync every 5 minutes, on internal signaling, on pull-to-refresh, on account switch and after a failed sync, per the server's guidance in docs/conversation.md.
Sends nothing on servers below conversation API v4, which answer in full.

The one thing to check in review

A delta response must never reach determineLeftConversationIds. That function lists every previously known conversation absent from the response — on a filtered response, nearly all of them — and the delete cascades through the foreign key to cached messages and chat blocks. RoomListResult.wasDelta is reported by the network layer rather than derived from the caller's intent, because an old server silently answers a delta request in full, and the reconcile is skipped on it.

Not in this PR

Periodic background refresh, and the foreground refresh loop while the list is open — stacked on top of this branch.
A full refresh on delete / disinvite signaling messages, which the server also recommends. Android handles neither event today.

🚧 TODO

Manual check against a real server: federated conversations are not skipped by delta syncs

🏁 Checklist

⛑️ Tests (unit and/or integration) are included or not needed
🔖 Capability is checked or not needed
🔙 Backport requests are created or not needed: /backport to stable-xx.x
📅 Milestone is set
🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

The content of this PR was partly or fully generated using AI

github-actions · 2026-09-21T14:20:44Z

📱 QA build


Download	app-qa-debug.apk
QR code	Open the QR code for this download
Commit	`0a1d95f`
Version	`6744`
Available until	7 days after this build

The QA build installs alongside a released Nextcloud app, so you can keep
using your existing install while testing.

Downloading the file requires a GitHub account, so open this link on the
device you want to test on, or transfer the APK to it.

coderabbitai · 2026-10-01T06:24:52Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: df09f746-6ffa-498c-80c6-2eeb011b9ecd

📥 Commits

Reviewing files that changed from the base of the PR and between ebf3e28 and 96fd5ad.

📒 Files selected for processing (7)

app/src/main/java/com/nextcloud/talk/api/NcApi.java
app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt
app/src/main/java/com/nextcloud/talk/jobs/DeleteConversationWorker.java
app/src/main/java/com/nextcloud/talk/jobs/LeaveConversationWorker.kt
app/src/test/java/com/nextcloud/talk/conversationlist/data/network/ConversationListDeltaSyncIntegrationTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

Room-list fetching now supports server delta responses with modification timestamps. The repository persists sync state, preserves omitted conversations for delta responses, and reconciles full responses. Pull-to-refresh, successful conversation leave, and successful conversation deletion request a full sync. The network layer parses the modification header and throws for unsuccessful HTTP responses. Tests cover delta preservation, full-sync reconciliation, and timestamp handling.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 96fd5

No new merge-blocking issue is established. The planned real-server check, including federated conversations, remains useful before release.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 96fd5

Delta responses are correctly prevented from deleting conversations they omit, and synchronization remains account-scoped. However, overlapping requests can undermine leave/delete cleanup: an older response can restore stale conversation data and reset synchronization state after a newer refresh. Recovery then depends on another full refresh.

Retained concerns

Medium · reliability · inferred: Full-sync invalidation does not dominate older synchronization commits. Each fetch launches independently, while requireFullSync only clears the last-full-sync timestamp. An older full response can commit after invalidation or a newer forced refresh, reinsert a removed conversation, and restore synchronization timestamps. Later deltas preserve that omitted conversation until another full reconciliation. Request overlap existed before this PR, but the new delta flow weakens its cleanup and recovery behavior. This affects local cache lifecycle and failure containment; it does not establish a server authorization bypass.

Security review details

Security Blast Radius

inferred — The identified ordering failure can affect conversation rows and displayed metadata within the synchronized account's local cache. The inspected evidence does not establish cross-account access, additional server privileges, or an authorization bypass.

Trust Boundaries and Controls

observed — Remote room data and the server-provided modification header influence local cache updates and future request selection through the existing authenticated API client. The cursor is parsed as a positive integer, and delta requests are gated by API version. The changed client declaration retains the Authorization header.

Resilience and Maintainability Implications

inferred — Database-before-cursor ordering contains sequential failures: interruption before cursor persistence leaves an earlier cursor available for replay rather than advancing past uncommitted data. This ordering does not provide protection against obsolete concurrent commits or invalidation races.

Hardening Proposals

proposed — Make synchronization commits and full-sync invalidation obey an account-scoped ordering invariant. For example, serialize transitions and reject responses from an earlier invalidation generation before applying database changes or advancing timestamps.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 85 functions across 17 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change: delta synchronization requests only conversations changed since the previous sync.
Description check	✅ Passed	The description explains the implementation, sync conditions, safeguards, tests, scope, TODO, and checklist status. The template's screenshots section is absent, but it is not critical for this non-vi…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b7669712-a578-4858-a89e-9076348f8034

📥 Commits

Reviewing files that changed from the base of the PR and between ab89e1d and 103dc8b.

📒 Files selected for processing (15)

app/src/main/java/com/nextcloud/talk/api/NcApi.java
app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/network/ConversationsNetworkDataSource.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/network/RetrofitConversationsNetwork.kt
app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt
app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt
app/src/main/java/com/nextcloud/talk/data/database/dao/ConversationsDao.kt
app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt
app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt
app/src/test/java/com/nextcloud/talk/conversationlist/data/network/ConversationListDeltaSyncIntegrationTest.kt
app/src/test/java/com/nextcloud/talk/conversationlist/data/network/ConversationListFreshnessIntegrationTest.kt
app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt
app/src/test/java/com/nextcloud/talk/conversationlist/data/network/RoomListMessagePrefetchIntegrationTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

coderabbitai

Actionable comments posted: 2

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: c76b06e8-90ba-4bf0-ad1c-eb7f39d266fe

📥 Commits

Reviewing files that changed from the base of the PR and between 103dc8b and cbca482.

📒 Files selected for processing (9)

app/src/main/java/com/nextcloud/talk/api/NcApi.java
app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/network/ConversationsNetworkDataSource.kt
app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt
app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt
app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt
app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt
app/src/test/java/com/nextcloud/talk/conversationlist/data/network/ConversationListFreshnessIntegrationTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Add the modifiedSince query parameter to GET /room and return the full response so the X-Nextcloud-Talk-Modified-Before header can be read. The network data source now reports the conversations, the timestamp to use for the next delta request, and whether the response it got back was a delta at all - a server below conversation API v4 answers in full whatever it was asked, and a caller that assumed otherwise would reconcile deletions against the wrong kind of response. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>

Send the timestamp the server handed back on the last conversation list sync as modifiedSince, so a sync carries what changed rather than every room with its last message, and ask without includeStatus while doing so - with it the server returns every one-to-one room whatever modifiedSince says, which is most of the list on a typical account. The response to such a request cannot express a conversation being deleted or the user being removed from one, so the left-conversation reconcile is skipped for it: every conversation the response leaves out would otherwise look like one that was left, and deleting those cascades through the foreign key to their cached messages and chat blocks. The network layer reports whether the response really was a delta, because a server below conversation API v4 answers in full whatever it was asked. The timestamp is stored per account in arbitrary storage, and only once the response is in the database - one kept ahead of a write that then failed would permanently skip the conversations that write was carrying. A failed sync drops it, so the next one asks for everything again. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>

mahibi · 2026-10-02T11:38:40Z

when leaving a conversation on mobile, it will pop up again when the modifiedSince requst is made and will only be gone after the full sync is done.
deleting a conversation on mobile -> it remains visible until the next full sync

in these cases, trigger a full sync

AndyScherzinger · 2026-10-02T15:51:21Z

Both confirmed and fixed in f2b3a02b7 (code) and c0d949b56 (test), amended into their originating commits.

Same root cause for both: the app removes the conversation on the server and leaves the local row to the list sync's reconciliation. A modifiedSince response cannot express a removal, so the reconciliation is deliberately skipped for it, and the row survived until the next full sync fell due.

Rather than fixing the two call sites in the conversation list, the invalidation sits in the two workers, so leaving or deleting from the chat screen and from conversation info are covered as well:

/** Makes the next conversation list sync of the account with the internal id [accountId] fetch the whole list. */
fun requireFullSync(accountId: Long)

LeaveConversationWorker and DeleteConversationWorker call it once the server has confirmed the removal, which clears the stored conversation_list_last_full_sync_at and so forces the next sync — whichever one runs first — to be a full one.

On top of that, the list now refreshes immediately in both cases instead of waiting: leaving already called fetchRooms() and now passes forceFullSync = true, and deleting did not refresh at all, which is why it stayed visible the longest.

Covered by a conversation left on this device is gone after requireFullSync in ConversationListDeltaSyncIntegrationTest, verified to fail when requireFullSync is neutered.

Still worth a retest on device, since I could only reproduce the mechanism in tests, not the UI flow.

A modifiedSince response cannot say that a conversation was deleted or that the user was removed from one, so the server asks clients to fetch the whole list regularly anyway. Follow that: at least every five minutes, and always when the internal signaling backend is in use, where no signaling server exists to announce the change out of band. A caller can also demand one. Pull to refresh does, so a conversation left on another device is gone by the time the indicator stops spinning rather than within the next five minutes. A device clock that moved backwards makes the last full sync read as being in the future; that is not a young full sync but an unusable one, and it asks for a full one too. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>

The case worth a test here is the one that destroys data rather than the one that annoys: a delta response lists only what changed, so every conversation it leaves out looks like one the user left, and reconciling those away takes their cached messages and chat blocks with them through the foreign key cascade. The first test seeds ten conversations with cached chat, runs a delta sync that mentions one of them, and asserts the other nine still have theirs. It was checked against the mistake it guards: with the delta branch removed from the sync, it fails. The rest cover what decides the mode - the stored timestamp is sent and includeStatus is not, the internal signaling backend never gets a delta, a full response still reconciles a conversation away, and a failed sync drops the timestamp so the next one asks for everything. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>

The per-room message catch-up wraps each room in runCatching, which also catches the CancellationException raised when the sync is stopped. The failure was logged and the loop moved on to the next room, so a sync that had been cancelled kept issuing requests for every remaining room. Rethrow the cancellation so the loop ends with the scope that owns it. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>

github-actions · 2026-10-02T19:01:57Z

Codacy

Lint

Type	master	PR
Warnings	138	138
Errors	18	15

SpotBugs

Category	Base	New
Bad practice	7	7
Correctness	11	11
Dodgy code	40	40
Internationalization	3	3
Malicious code vulnerability	3	3
Performance	8	8
Security	11	11
Total	83	83

github-actions Bot added the AI assisted label Sep 21, 2026

This was referenced Sep 21, 2026

Refresh conversations in the background so they are not stale when the app is opened #6745

Open

Refresh the conversation list while it is open #6746

Open

AndyScherzinger added this to the 25.1.0 milestone Sep 21, 2026

AndyScherzinger added the 2. developing Work in progress label Sep 21, 2026

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch 5 times, most recently from 0b93e1e to 465e035 Compare September 23, 2026 12:26

AndyScherzinger mentioned this pull request Sep 23, 2026

Mark message prefetch requests and let the server turn preloading off #6763

Open

6 of 7 tasks

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch 14 times, most recently from cdd211e to efe3157 Compare September 29, 2026 22:47

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch from efe3157 to 103dc8b Compare October 1, 2026 06:20

AndyScherzinger marked this pull request as ready for review October 1, 2026 06:20

AndyScherzinger added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 1, 2026

AndyScherzinger self-assigned this Oct 1, 2026

AndyScherzinger requested a review from mahibi October 1, 2026 06:21

coderabbitai Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread ...c/main/java/com/nextcloud/talk/conversationlist/data/network/RetrofitConversationsNetwork.kt

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch from 103dc8b to cbca482 Compare October 1, 2026 19:24

coderabbitai Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread ...java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt

Comment thread ...java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch from cbca482 to 372c35b Compare October 1, 2026 20:32

AndyScherzinger added 2 commits October 2, 2026 11:57

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch from 372c35b to ebf3e28 Compare October 2, 2026 09:57

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch from ebf3e28 to 96fd5ad Compare October 2, 2026 15:51

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch from 96fd5ad to 19a8ef6 Compare October 2, 2026 16:23

AndyScherzinger added 3 commits October 2, 2026 19:01

AndyScherzinger force-pushed the feat/noid/conversation-list-delta-sync branch from 19a8ef6 to 0a1d95f Compare October 2, 2026 17:01

Navigation Menu

Uh oh!

Ask the server only for the conversations that changed - #6744

Ask the server only for the conversations that changed#6744
AndyScherzinger wants to merge 5 commits into
masterfrom
feat/noid/conversation-list-delta-sync

AndyScherzinger commented Sep 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 21, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

Reviews paused

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

mahibi commented Oct 2, 2026 •

edited by AndyScherzinger

Loading

Uh oh!

AndyScherzinger commented Oct 2, 2026

Uh oh!

github-actions Bot commented Oct 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

AndyScherzinger commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What it does

The one thing to check in review

Not in this PR

🚧 TODO

🏁 Checklist

🤖 AI (if applicable)

Uh oh!

github-actions Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📱 QA build

Uh oh!

coderabbitai Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mahibi commented Oct 2, 2026 • edited by AndyScherzinger Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyScherzinger commented Oct 2, 2026

Uh oh!

github-actions Bot commented Oct 2, 2026

Codacy

Lint

SpotBugs

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AndyScherzinger commented Sep 21, 2026 •

edited

Loading

github-actions Bot commented Sep 21, 2026 •

edited

Loading

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

mahibi commented Oct 2, 2026 •

edited by AndyScherzinger

Loading