I admit this patch is quite tricky. Please try to follow me.
So, first off, in `SlidingSyncRoom::new`, we were clearing the timeline,
because somehow it exists twice in memory at this step.
Which led me to understand how `SlidingSync::handle_response` was working.
I've clarified how this part of the code works. We are dealing with 2 kind of
responses for a specific reason: `SyncResponse` and `v4::Response`, now it's
documented and I hope it's clearer.
Then, I notice that we were passing a clone of the entire sliding sync
response (`v4::Response`) to `Client::process_sliding_sync`. I thought it was
suboptimal, so I've updated the code to take a reference. It led me to update
`BaseClient::process_sliding_sync`. It was a little bit tricky, but I reckon we
have less clones now than before.
And now, back to `SlidingSync::handle_response`, I was able to compute the
`timeline` correctly by draining it from the `v4::Response`, or by moving it
from `SyncResponse`. So it's no longer necessary to have this clearing code
inside `SlidingSyncRoom::new`. Honestly it has nothing to do at this place
before.
To conclude: We have cleaner code, and less clones.
What thing I reckon could be optimized, is that the entire `timeline`
(`Vec<TimelineEvent>`) is cloned to be passed to `Client::handle_timeline`. So
this timeline exists in 2 places: in Sliding Sync, and somewhere else. I don't
believe it's a problem now, that's how it works, but we must be aware of that.
The `Client::login_custom` allows to login by using a custom login
method. In particular, it is possible to login to Synapse which supports
JWT authentication.
Signed-off-by: boxdot <d@zerovolt.org>
First off, it's not necessary for `SlidingSyncRoom::update` to take a reference
to `room_data: v4::SlidingSyncRoom`. When `update` is called, the iterator owns
its items.
Second, by taking ownership of `room_data`, we no longer need to clone all the
data we need to assign to `self.inner`.
First off, `SlidingSyncRoom.from` doesn't need to be visible to the entire
crate, only to `crate::sliding_sync.
Second, it's a constructor, so let's call it `new`.
1. Put `FrozenSlidingSyncRoom` at the bottom of the module.
2. Put merge 2 `impl SlidingSyncRoom` together.
3. Remove the `AliveRoomTimeline` type alias.
4. Improve the documentation.
So far, the `SlidingSync.pos` field was public to the crate. In order to avoid
breaking the internal state of this type, its visibility is now private.
However, we need to be able to change the value when testing the
`SlidingSync` type itself. To achieve that, this patch removes the old
`force_sliding_sync_pos` function, and implements 2 new functions: `set_pos`
and `pos` directly on `SlidingSync` only when `#[cfg(any(test, feature ="testing"))]`.
Previously, it was possible for us to use invalid indices when:
- We retried decrypting multiple events at once
- One of them (not the last) was an edit or reaction
This lead to a situation where we would remove the UTD item once
decryption for it was successfully retried, but not account for the
resulting index shift for all later timeline items.
Benjamin Kampmann