Caching toots - attempt 2

[charlag]

I've tried to rebase #663 but it was branched from another branch and it is too old so I failed.
Instead I've cherry-picked last 5 commits from that branch which seem to contain useful changes.
I did not try it yet because I'm out of energy already but it can be useful for someone to inspect or fix.

[charlag]

I've updated PR to fix some errors. At least some caching is working now but I am still not sure how to mark gaps in DB

[connyduck]

The offline part works, very well done.
Three major bugs immediately caught my eye: Can't scroll timeline more than one page anymore, database migration is missing & multiple accounts get their timelines mixed up.
Why can't you save a almost empty status (everything null) as placeholder?

[charlag]

Thanks for checking it out!

1. It was there but apparently broke, didn't test it now
2. It was there but got lost, again
3. Because I sort statuses by ID. Which ID should I insert for bogus status? Can I sort them by time? Who knows

[connyduck]

Because I sort statuses by ID. Which ID should I insert for bogus status? Can I sort them by time? Who knows

what if when a placeholder is encountered, we only save the upper part of the timeline?

[charlag]

And delete everything below? That would be unpleasant

[connyduck]

Yes, but it should be a cache and not a save-the-whole-timeline-forever thing, shouldn't it?

Another possible solution could be to use one of the ids that presumably belong inside the gap to the placeholder? The id of the toot above the gap minus 1 or something?

[charlag]

Uuugh, maybe my problem is that I was trying to do exactly this? With future search and everything.

You proposal could lead to clashes easily, especially on small instances and I would rather not.
Maybe we need a flag or something like that? Like "gap below"

[connyduck]

It definitely needs some clean up logic to prevent database size to grow forever

[charlag]

Yup, but that's another problem and it's pretty easy to solve imo

[charlag]

multiple accounts get their timelines mixed up

not sure why it could happen because WHERE s.timelineUserId = :account
🤔

[charlag]

I've tried adding a migration but didn't really check it yet.
I've pretty much just copied it from the scheme and checked in the scheme itself because... it's a good practice and makes your life easier I guess? Can remove it if there will be objections but I think it's not such a bad idea (bonus: can test migrations with in-memory DB. At least I think so)

[charlag]

Some progress on this
I've changed caching strategy. Now initial load is DB-only, after that data is fetched from the server to show updated content.
For all other requests it fetches from network but falls back to disk. Maybe there should be a better way than catching IOExceptions. It doesn't fix "gap problem" but makes it "offline only" which is acceptable IMO and it seems like Pinafore works like that too.

I've also fixed some problems with duplicated DB entries. Mixing statuses between accounts should be fixed now but I didn't check. Need to save some more account fields to work correctly.

There's a hack for the initial request which asks for statuses with margin of 11 which seems to be working but maybe not always and we should figure this out.

We can improve thread loading by not reloading the toot itself now when we have it in the DB.

[charlag]

Also not forget to create an index on timeline account entities as Room proposes, otherwise updates will be slow

Also don't forget to update statuses on events

[charlag]

Rebased on master, fixed couple of small things, will check how it'll work.
(actually works kinda weird because on pull we scroll to top)
Now I'm trying to get current statuses but I keep newer statuses too.
I'm kinda lost, not sure what's missing now. Would appreciate some feedback but no pressure.

[charlag]

OK, first pull-to-refresh doesn't work, uuugh

[charlag]

Works better now, maybe we can really get there.
I think that we should do two requests on startup now: one for refreshing cached list and another one to get the latest things (to insert them on top). Will try to try that tomorrow

[charlag]

Hey, I know I've said that you can review it but I already see some conceptual problems.
Users want to retain their position but we don't do that and we can't actually do that until we have some way to mark (damn) gaps. Maybe idea with empty status is not so bad and we can do that. Will try to describe today but it may be tricky still.

[charlag]

As I said, I think we still need to implement gaps indicator.
Rough idea on how they would work:
We have three cases: loading top, loading middle, loading bottom (we skip initial for now).

Top: we check that the oldest of returned statuses to us is a known to us. If not, but a gap into DB with
(oldest of returned) < id (newest of known).

Middle: we do the same check for both top and bottom. If we did not overlap with known statuses on one edge, we insert gap on that edge with ID in the middle.

Bottom: we always insert gap below known things. We fill the gap if no statuses are returned (server cannot give them to us).

So far I don't see a problem with this approach. Id of the gap should always be an id between real statuses we know about. When we load statuses in the gap, we either replace the gap with a status with the same ID or we don't and the gap is still there. We should think if we should should prevent double gaps and how to do that (we don't want to see two gaps together in a timeline e.g.)

I'm sorry that I dismissed that idea before, now I think that it may even work.

[connyduck]

I tested a build from commit 7f0836e. Here is what I noticed:

• Migrations are not working
• After deleting data everything is mostly fine
• when killing app and restarting, the timeline is jumping in a weird way (I can make a video if you want)
• when pulling to refresh and there are new toots loading, timeline scrolls up way more than previously

[charlag]

Thx for testing! I think most of it should be solved by the next commit now but we will need to re-test anyway

[charlag]

Uff, much stuff today.
I think most things are in place. Biggest problems are:

• Sometimes there are two placeholders next to each other and they are broken in this case
• Almost no tests

[charlag]

One of the biggest remainig problems is "too many placeholders" (thing are not so bad if it's a problem I think).
While for toots from DB it is acceptable (but we still may avoid it) the most annoying one is one on the top which we get every time for some reason.
Once we solve this (and migration) we should be done.

We should discuss if we want to include saving position in this PR or implement it later. It shouldn't be a big problem but this is already huge.

[charlag]

I've spent quite some time trying to find out why timeline Mastodon API is acting so weirdly.
I'm not sure yet if this will help but seems so.
Until it's widespread we will have do deal with some extra placeholders (or spam the server but it's harder).

@connyduck can I have your opinion on including position remembering in this PR or not?

[connyduck]

Installing new build right now, will give you feedback tomorrow!

Lets leave remembering position for now and focus on getting this merged, we can add it later. I think it should not be too much effort then.

[charlag]

Thanks! I won't be near PC for the weekend but next week I can apply feedback!
Yeah, it's not going to be much.
Maybe we should also add cleanup (something like deleting all toots older than two weeks or something? Maybe except own)

[connyduck]

It seems that migrations work now!
But ... the home timeline does not load anymore, it stays on the progress bar forever ._.

[charlag]

😱