timeline/sendq: add a Timeline::redact() method and tweak local echoes behavior
#3488
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ground This implements the following features: - enabling/disabling the sending queue at a client-wide granularity, - one sending queue per room, that lives until the client is shutting down, - retrying sending an event three times, if it failed in the past, with some exponential backoff to attempt to re-send, - allowing to cancel sending an event we haven't sent yet. fixup sdk
A client would require this to know that the sending queue has been globally disabled, across all the rooms, otherwise they couldn't know that it needs to be re-enabled later on.
So as to not use sync mutexes across await points, we have to use an async mutex, BUT it can't be immediately called in a wiremock responder, so we need to shoe-horn a bit: create a new tokio runtime, which can only be called from another running thread, etc. It's a bit ugly, but I couldn't find another mechanism to block the responder from returning a Response immediately; happy to change that anytime if there's a simpler way.
…al echoes fixup timeline
Technically, the test already passed before the change in the builder, because `TimelineEventHandler::handle_event` already filters out local events on non-live timelines, but it's a waste of resources to even spawn the local echo listener task in that case, so let's not do it.
…cal echo The test relied on the fact that sending an event from a given timeline is not observable from another timeline. Indeed, it sent a message using a first timeline object, then constructed a second timeline object and expected only the remote event to be in there. Now, the sending queue is shared across all instances of a Room, thus all instances of a Timeline, and the second timeline can see the local echo for the message sent by the first timeline. The "fix" is thus in the test structure itself: when waiting for the remote echo to be there, check that the timeline item doesn't pertain to a local echo, i.e. is a remote echo.
It was used after a previous local echo couldn't be sent (i.e. the sending queue failed to send it). However, it was slightly incorrect to mark those as cancelled, since those local echoes would still have corresponding items in the sending queue, and would be retried as soon as the sending queue was reenabled and could send events. Instead, this is removed: it means that previously cancelled events would be in the NotSentYet state, which is correct. (At the limit, we could even get rid of the SendingFailed variant, since the sending queue will automatically retry as soon as possible.)
Also reunify two methods that did the same thing, with slightly different semantics, and test the one that wasn't tested at all. Note that `is_editable()` was already exposed to the FFI and used in EX apps.
bnjbvr
force-pushed
the
bnjbvr/send-q-timeline-improvements
branch
from
78d5984 to
84695ad
Compare
|
(Closing, #3475 contains all the changes now.) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moar changes requested by @stefanceriu, please test :)
Room::redactsince it can have an effect on local echoes too.Part of #3361.