Feature/fga/simplify timeline logic #6318
Conversation
| val chunkEntity = if (!isLimited && lastChunk != null) { | ||
| lastChunk | ||
| } else { | ||
| // Delete all chunks of the room in case of gap. | ||
| ChunkEntity.findAll(realm, roomId).forEach { | ||
| it.deleteOnCascade(false, canDeleteRoot = true) |
There was a problem hiding this comment.
canDeleteRoot = true is there a risk to delete state event here? Or Event needed for aggregation?
There was a problem hiding this comment.
It will delete everything except state events
ganfra
requested review from
a team,
Claire1817,
mnaturel and
bmarty
and removed request for
a team
...roid/src/main/java/org/matrix/android/sdk/internal/database/migration/MigrateSessionTo030.kt
Outdated
Show resolved
Hide resolved
…al/database/migration/MigrateSessionTo030.kt Co-authored-by: Benoit Marty <benoitm@matrix.org>
| chunk.getList(ChunkEntityFields.TIMELINE_EVENTS.`$`).clearWith { timelineEvent -> | ||
| // Don't delete state events | ||
| if (timelineEvent.isNull(TimelineEventEntityFields.ROOT.STATE_KEY)) { | ||
| timelineEvent.getObject(TimelineEventEntityFields.ROOT.`$`)?.deleteFromRealm() |
There was a problem hiding this comment.
I see there may be also other entities attached to the event such as: EventAnnotationsSummaryEntity and ReadReceiptsSummaryEntity. Is it okay to keep them?
There was a problem hiding this comment.
Yes we can keep them, they are kind of independent
| @@ -445,7 +445,7 @@ internal class TimelineChunk( | |||
| Timber.e(failure, "Failed to fetch from server") | |||
| LoadMoreResult.FAILURE | |||
| } | |||
| return if (loadMoreResult == LoadMoreResult.SUCCESS) { | |||
| return if (loadMoreResult != LoadMoreResult.FAILURE) { | |||
There was a problem hiding this comment.
Just checking, is it intended to call loadMore when result is LoadMoreResult.REACHED_END?
There was a problem hiding this comment.
This is the downside of linking the last permalink chunk to the lastForward chunk... I still don't know if we want this or just add timeline events from sync on both chunks instead
|
SonarCloud Quality Gate failed. |
* Sync: delete all previous chunks in case of gappy sync * Chunk: dont link chunks if we find existing timeline event (keep multiple timeline events in db) * Timeline : remove some unused code * Clean and add changelog * Timeline: set named argument * Timeline: avoid restarting the timeline when there is a CancellationException due to permalink * Timeline: add migration to clean up old (broken) chunks * Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/migration/MigrateSessionTo030.kt Co-authored-by: Benoit Marty <benoitm@matrix.org> * Timeline: try to fix test * ignoring broken instrumentation test in order to release Co-authored-by: ganfra <francoisg@element.io> Co-authored-by: Benoit Marty <benoitm@matrix.org> Co-authored-by: Adam Brown <adampsbrown@gmail.com>
| @@ -0,0 +1 @@ | |||
| Fix loop in timeline and simplify management of chunks and timeline events. | |||
There was a problem hiding this comment.
Does this also close the following? They seem like duplicates of #5166 anway
Type of change
Content
Simplify the management of timeline events and chunks :
Motivation and context
There was multiple scenarios where we had missing messages or loops in timeline.
This fixes #6312 and fixes #5166
Tested devices
Checklist