runbox7 icon indicating copy to clipboard operation
runbox7 copied to clipboard

Unread message counters are unreliable

Open tadzik opened this issue 4 years ago • 3 comments

The counters for unread messages on the folder list are inconsistent with the message list – and reality :) In the best case they are severely delayed – in the worst, they're completely wrong. I'll to explain why (I think) this happens, and what we can hopefully do to make it better.

The reason why this happens is because they reflect the numbers returned by https://runbox.com/rest/v1/email_folder/list. That API call gives us the structure of the folder tree, but also the basic message counters tracked by IMAP (among other things?) and updated every now and then. The good news is that these are then mostly correct whenever a new message arrives and the account is reindexed (or so I think).

Problems start when the user marks the message as read. This triggers the API call to mark the message as read on the server here, which triggers a chain of events that ends up with the folder list being refreshed here.

If I'm reading this correctly (and my reading comprehension can be so-so when it comes to weird rxjs chains) this may already cause a race condition where we refresh folder counts before the message is successfully marked as read – but that's a minor thing in the grand scheme of things.

The thing worth noting is that our folder counters are always server-based and updated and stored in the same place the folder structure is. This becomes much more problematic when we use the local index for emails. While the local index doesn't contain the folder structure, it does contain the message flags, including the Seen flag. What the user sees as read/unread messages on the message list comes from here, and this is what can most easily diverge from what the folder list chooses: since the index may (and probably will) update much quicker than an API call that reads the folder data from the server database.

I think that the message list should be authoritative in terms of the folder counters: knowing that there are unread messages on the folderlist when there aren't any in the message list is useless knowledge anyway. It's easier said than done in case of online index, but with a local index it sounds relatively straightforward, and I think that's where we should focus on putting this.

Now, when I say “straightforward” I mean conceptually: our code isn't really receptive to this change in its current state. Like I mentioned before, the folder list fetched from the server contains the number of unread messages, and that's what the folder list displays. For this little “straightforward” revolution to work we'll need to uncouple the idea of the folder tree form the folder counters so that they can be updated and managed independently: in case of local index, the indexer itself will be in charge of updating the counters, not the API like it currently works.

I started some work on that locally. So far I did some refactoring of folderlist.component.ts, making it a lot dumber. My idea is to make it a mere “displayer” of the folders (and message counts) which delegates all the folder tree actions (creating, moving etc) to someone else. We can then prevent it from making misinformed decisions about who should be handling what (it now delegates some folder actions to rmmapi that is DI'd and some to messagelist which is set in its runtime :|). I'm aiming at making it as “functional” as Angular allows, which should both make it a lot easier to understand and test and also will allow us to feed it folder data that we calculate elsewhere, without making the folderlist understand all the intricacies of local index and whatnot. That part is about 50% done.

The next step will be to make a logical separation of the folder structure and the folders' aggregated contents. The former will still come from the API, while the latter can be calculated offline from the local index if it's available. This should eliminate the problems with mismatched numbers that we currently have, at least if the local index is used. While doing this I hope to do some aggressive refactoring of the things I go through, since plenty of cruft has accumulated over time while reality and requirements have changed somewhat :)

tadzik avatar Apr 06 '20 11:04 tadzik

The folder counts have been worked on many times since this issue was created, here's the current state (still not perfect). It would appear most of what tadzik described above, has been done at some point.

Folder info (names, nesting, counts etc) is pulled from the API, based on whether this has changed since we last fetched it, we trigger a folderCount update - despite the fact that the unread/total counts exist in the API data we just pulled, we're asking the index for this data.

Because each index update is: Update index, then refresh folder list (which causes possibly the foldercount to update) we're actually getting the following issue:

  1. update index - no changes
  2. refresh folder list - this could find a change that happened since the update index ran, literally seconds before
  3. folder list changing triggers the folderCount update - this doesnt change as its counting from the index thats the same as last time
  4. update index runs again - finds changes, updates the index
  5. refresh folder list runs again - finds NO changes (cos we dealt with them earlier), DOESNT trigger the folder count update - so now the count is wrong.

Current state partly due to my recently changing things so that the foldercounts are only redone when the folder list changes (else we spend many seconds redrawing the folder list UI pointlessly) partly due to the fact that we're basing the "has it changed" on the API data, but counting from the index - these need to match to make sense!

My preferred way to fix this would be to display counts from the API, not the index ..

NB: This issue occurs when users are using RMM6 and/or IMAP, as well as RMM7 - its changing data outside of rmm7 itself that triggers it (which ought to be allowed, of course..)

castaway avatar Sep 23 '21 12:09 castaway

Could some of these cases be related to messages marked as deleted via IMAP?

Ref: https://community.runbox.com/t/rb7-looks-bad-here-is-how-i-would-improve-it/1344/16

gtandersen avatar Jan 24 '22 00:01 gtandersen

It is possible that imap-marked-as-deleted is confusing things:

Our folder counts are based on total/unseen counts in folders, this also counts anything marked as deleted, but not yet deleted (it doesnt query the flag at all). I think the messagelist display does hide the marked as deleted items, so that would be conflicting.

castaway avatar Jun 29 '22 14:06 castaway