feat: keep timeline history for archive rooms in memory - [merged] #1380

Closed famedly-bot closed 1 year ago

In GitLab by @h.carnot on Apr 8, 2022, 11:54

Merges henri/fix-timeline-history -> main

Description

Fixes #275 #311

For archived rooms, we don't want to store the events in the database. For this, we store the sync response containing all the timeline for the archived rooms in memory and ask explicitly the sync handler to not update the database.

TODO

[x] : Initial implementation
[x] : Rewrite requestHistory to not save requested events in the database for archived rooms.
[x] : Verify that we can decrypt the events without needing to save the event update.
[x] : What if we are suddenly re-invited in a archived room. See if the timeline is properly discarded.
[x] : Provide a way to discard _archivedRooms. What if this take too much ram ?
[x] : Tests

Checklist

[ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
[ ] I updated/added relevant documentation (doc comments with ///).
[ ] I did not add or update methods, functions or widgets without tests.

In GitLab by @h.carnot on Apr 8, 2022, 11:55

result

We may have an issue with the way the timeline is sorted :D

In GitLab by @h.carnot on Apr 8, 2022, 12:40

added 1 commit

accf996c - feat: keep timeline history

Compare with previous version

In GitLab by @h.carnot on Apr 8, 2022, 13:52

added 1 commit

685829e1 - feat: handle sync response

Compare with previous version

In GitLab by @h.carnot on Apr 8, 2022, 13:56

Commented on lib/src/client.dart line 834

option 1

In GitLab by @h.carnot on Apr 8, 2022, 13:56

Commented on lib/src/client.dart line 795

option 3

In GitLab by @h.carnot on Jun 2, 2022, 17:56

Commented on lib/src/client.dart line 795

changed this line in version 4 of the diff

In GitLab by @h.carnot on Jun 2, 2022, 17:56

added 1 commit

701d9e2e - fix: don't discard from memory archived rooms

Compare with previous version

In GitLab by @h.carnot on Jun 2, 2022, 18:09

added 1 commit

aba23437 - fix: disable storing archived rooms in database

Compare with previous version

In GitLab by @h.carnot on Jun 2, 2022, 18:10

resolved all threads

In GitLab by @h.carnot on Jun 2, 2022, 18:21

marked the checklist item : Rewrite requestHistory to not save requested events in the database for archived rooms. as completed

In GitLab by @h.carnot on Jun 3, 2022, 11:47

added 1 commit

48c91af3 - fix: added tests and allow discarding archived rooms from memory

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 11:47

marked the checklist item : Provide a way to discard _archivedRooms. What if this take too much ram ? as completed

In GitLab by @h.carnot on Jun 3, 2022, 11:47

marked the checklist item : Tests as completed

In GitLab by @h.carnot on Jun 3, 2022, 12:07

added 2 commits

73e950e3 - fix: added tests and allow discarding archived rooms from memory
9435abaa - fix: rename field and less strict controll on what can be saved

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 12:08

added 1 commit

0c52d529 - fix: format test

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 13:55

marked the checklist item : Verify that we can decrypt the events without needing to save the event update. as completed

In GitLab by @h.carnot on Jun 3, 2022, 14:08

added 1 commit

d4da02a7 - fix: discard room from archive list if membership changed

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 14:16

added 1 commit

340f6e73 - fix: add test for discard from archives when membership change

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 14:16

marked the checklist item : What if we are suddenly re-invited in a archived room. See if the timeline is properly discarded. as completed

In GitLab by @h.carnot on Jun 3, 2022, 14:39

added 86 commits

340f6e73...f24c5517 - 76 commits from branch main
cbc7b964 - feat: keep timeline history
a181763b - feat: handle sync response
a53930ea - fix: don't discard from memory archived rooms
6444bf4b - fix: disable storing archived rooms in database
507e225e - fix: added tests and allow discarding archived rooms from memory
4750ef55 - fix: rename field and less strict controll on what can be saved
57823406 - fix: format test
6422b142 - fix: discard room from archive list if membership changed
2a1dad76 - fix: add test for discard from archives when membership change
78aedb94 - fix: update

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 14:40

added 1 commit

ffbc4166 - fix: format

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 14:47

added 1 commit

68008775 - fix: revert modifications

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 15:26

added 1 commit

1df35ffa - feat: keep timeline history

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 15:35

added 1 commit

41926d45 - feat: keep timeline history

Compare with previous version

In GitLab by @h.carnot on Jun 3, 2022, 15:35

marked this merge request as ready

In GitLab by @h.carnot on Jun 3, 2022, 15:35

This also fix a bug where the event order was wrong for left rooms.

In GitLab by @krille-chan on Jun 7, 2022, 08:42

requested review from @krille-chan

In GitLab by @krille-chan on Jun 7, 2022, 08:45

Commented on lib/src/client.dart line 786

Some dartdocs would be useful here so the user can understand the difference better between this method and loadArchiveWithTimeline()

In GitLab by @krille-chan on Jun 7, 2022, 08:46

Let me try this out in FluffyChat :)

In GitLab by @h.carnot on Jun 7, 2022, 11:38

added 1 commit

83671a4c - fix: add dart docs

Compare with previous version

In GitLab by @h.carnot on Jun 7, 2022, 11:38

Commented on lib/src/client.dart line 786

done :)

In GitLab by @h.carnot on Jun 9, 2022, 17:12

mentioned in issue famedly-web#436

In GitLab by @h.carnot on Jun 9, 2022, 17:16

mentioned in issue famedly-web#338

In GitLab by @h.carnot on Jun 10, 2022, 13:28

Did it work ? :D

In GitLab by @nico-famedly on Jun 21, 2022, 11:27

Commented on lib/src/client.dart line 2814

Nit, but shouldn't this be an ArchivedRoom?

In GitLab by @nico-famedly on Jun 21, 2022, 11:27

Commented on lib/src/client.dart line 1705

Shouldn't we delete the room from the database at some point?

In GitLab by @nico-famedly on Jun 21, 2022, 11:27

Commented on lib/src/client.dart line 1616

What is this parameter for?

In GitLab by @nico-famedly on Jun 21, 2022, 11:27

In general this looks reasonable and tests all the cases I would want to see tested, but I have some questions :3

In GitLab by @h.carnot on Jun 21, 2022, 11:32

Commented on lib/src/client.dart line 2814

changed this line in version 17 of the diff

In GitLab by @h.carnot on Jun 21, 2022, 11:32

added 1 commit

a7e9ecee - fix: typo

Compare with previous version

In GitLab by @h.carnot on Jun 21, 2022, 11:33

added 28 commits

a7e9ecee...96c420fb - 27 commits from branch main
127b3996 - feat: keep timeline history

Compare with previous version

In GitLab by @h.carnot on Jun 21, 2022, 11:34

Commented on lib/src/client.dart line 2814

yep, no strong opinion about it

In GitLab by @h.carnot on Jun 21, 2022, 11:36

Commented on lib/src/client.dart line 1616

it's to allow storing left update in the database if needed. In my opinion, we should not provide a way to allow saving left room update in teh database

In GitLab by @h.carnot on Jun 21, 2022, 11:42

Commented on lib/src/client.dart line 1616

changed this line in version 19 of the diff

In GitLab by @h.carnot on Jun 21, 2022, 11:42

added 1 commit

4d3faba6 - fix: removed uneeded checks

Compare with previous version

In GitLab by @h.carnot on Jun 21, 2022, 11:45

Commented on lib/src/client.dart line 1705

it's deleted if we rejoin the room https://gitlab.com/famedly/company/frontend/famedlysdk/-/merge_requests/1006/diffs#9080bea85a12b6959b4103f35f0205ec96f221e5_1889_1937

Also it's possible to clear the archived rooms cache https://gitlab.com/famedly/company/frontend/famedlysdk/-/merge_requests/1006/diffs#9080bea85a12b6959b4103f35f0205ec96f221e5_790_801 but I would say it's the job of the client to do so when the user navigate out of the archived rooms page

In GitLab by @nico-famedly on Jun 21, 2022, 11:47

Commented on lib/src/client.dart line 1705

No, I mean, we should delete the joined room from the database and convert it to an archived room?

In GitLab by @h.carnot on Jun 21, 2022, 11:47

Commented on lib/src/client.dart line 1616

So I removed it, it wasn't needed

In GitLab by @h.carnot on Jun 21, 2022, 11:48

Commented on lib/src/client.dart line 1705

But when you leave a room it's already discarded from the database. And for the archived room, we need to load them before and we don't want them to be stored. It would take memory and sync time for nothing.

In GitLab by @h.carnot on Jun 23, 2022, 17:17

added 1 commit

9e755a38 - fix: use old CI

Compare with previous version

famedly / matrix-dart-sdk