From 501c457ea130481ba5b52d45d4d0ff37b8707964 Mon Sep 17 00:00:00 2001 From: Krille Date: Mon, 6 May 2024 14:38:23 +0200 Subject: [PATCH] refactor: delete not sent events without eventupdate stream workaround This fixes several problems. First sending a fake event through the onEventUpdate stream was not a good design and lead to problems if you expect a timeline event but then are unable to build the event from json. This now uses a new stream in the Client which is listened to in the timeline to delete an event which should be much more reliable. It also now throws an exception if deleting the event fails instead of returning true or false. A deprecation note is added. --- lib/src/client.dart | 3 ++ lib/src/event.dart | 37 +++++++------ lib/src/event_status.dart | 5 -- lib/src/timeline.dart | 94 ++++++++++++++++++--------------- test/event_test.dart | 10 ++-- test/timeline_context_test.dart | 2 +- test/timeline_test.dart | 2 +- 7 files changed, 78 insertions(+), 75 deletions(-) diff --git a/lib/src/client.dart b/lib/src/client.dart index ae77f37b..605c2c18 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -1312,6 +1312,9 @@ class Client extends MatrixApi { final CachedStreamController onGroupMember = CachedStreamController(); + final CachedStreamController onCancelSendEvent = + CachedStreamController(); + /// When a state in a room has been updated this will return the room ID /// and the state event. final CachedStreamController<({String roomId, StrippedStateEvent state})> diff --git a/lib/src/event.dart b/lib/src/event.dart index f18e0a85..6ebaf00f 100644 --- a/lib/src/event.dart +++ b/lib/src/event.dart @@ -322,27 +322,26 @@ class Event extends MatrixEvent { return receiptsList; } - /// Removes this event if the status is [sending], [error] or [removed]. - /// This event will just be removed from the database and the timelines. - /// Returns [false] if not removed. + @Deprecated('Use [cancelSend()] instead.') Future remove() async { - final room = this.room; - - if (!status.isSent) { - await room.client.database?.removeEvent(eventId, room.id); - - room.client.onEvent.add(EventUpdate( - roomID: room.id, - type: EventUpdateType.timeline, - content: { - 'event_id': eventId, - 'status': EventStatus.removed.intValue, - 'content': {'body': 'Removed...'} - }, - )); + try { + await cancelSend(); return true; + } catch (_) { + return false; } - return false; + } + + /// Removes an unsent or yet-to-send event from the database and timeline. + /// These are events marked with the status `SENDING` or `ERROR`. + /// Throws an exception if used for an already sent event! + Future cancelSend() async { + if (status.isSent) { + throw Exception('Can only delete events which are not sent yet!'); + } + + await room.client.database?.removeEvent(eventId, room.id); + room.client.onCancelSendEvent.add(eventId); } /// Try to send this event again. Only works with events of status -1. @@ -358,7 +357,7 @@ class Event extends MatrixEvent { }.contains(messageType)) { final file = room.sendingFilePlaceholders[eventId]; if (file == null) { - await remove(); + await cancelSend(); throw Exception('Can not try to send again. File is no longer cached.'); } final thumbnail = room.sendingFileThumbnails[eventId]; diff --git a/lib/src/event_status.dart b/lib/src/event_status.dart index 9bec269f..77f529bc 100644 --- a/lib/src/event_status.dart +++ b/lib/src/event_status.dart @@ -6,7 +6,6 @@ /// - synced: (event came from sync loop) /// - roomState enum EventStatus { - removed, error, sending, sent, @@ -33,7 +32,6 @@ EventStatus latestEventStatus(EventStatus status1, EventStatus status2) => extension EventStatusExtension on EventStatus { /// Returns int value of the event status. /// - /// - -2 == removed; /// - -1 == error; /// - 0 == sending; /// - 1 == sent; @@ -41,9 +39,6 @@ extension EventStatusExtension on EventStatus { /// - 3 == roomState; int get intValue => (index - 2); - /// Return `true` if the `EventStatus` equals `removed`. - bool get isRemoved => this == EventStatus.removed; - /// Return `true` if the `EventStatus` equals `error`. bool get isError => this == EventStatus.error; diff --git a/lib/src/timeline.dart b/lib/src/timeline.dart index 742b8e81..5934bf35 100644 --- a/lib/src/timeline.dart +++ b/lib/src/timeline.dart @@ -44,6 +44,7 @@ class Timeline { StreamSubscription? sub; StreamSubscription? roomSub; StreamSubscription? sessionIdReceivedSub; + StreamSubscription? cancelSendEventSub; bool isRequestingHistory = false; bool isRequestingFuture = false; @@ -278,6 +279,8 @@ class Timeline { sessionIdReceivedSub = room.onSessionKeyReceived.stream.listen(_sessionKeyReceived); + cancelSendEventSub = + room.client.onCancelSendEvent.stream.listen(_cleanUpCancelledEvent); // we want to populate our aggregated events for (final e in events) { @@ -291,6 +294,16 @@ class Timeline { } } + void _cleanUpCancelledEvent(String eventId) { + final i = _findEvent(event_id: eventId); + if (i < events.length) { + removeAggregatedEvent(events[i]); + events.removeAt(i); + onRemove?.call(i); + onUpdate?.call(); + } + } + /// Removes all entries from [events] which are not in this SyncUpdate. void _removeEventsNotInThisSync(SyncUpdate sync) { final newSyncEvents = sync.rooms?.join?[room.id]?.timeline?.events ?? []; @@ -306,6 +319,8 @@ class Timeline { roomSub?.cancel(); // ignore: discarded_futures sessionIdReceivedSub?.cancel(); + // ignore: discarded_futures + cancelSendEventSub?.cancel(); } void _sessionKeyReceived(String sessionId) async { @@ -462,55 +477,46 @@ class Timeline { : null) ?? EventStatus.synced.intValue); - if (status.isRemoved) { - final i = _findEvent(event_id: eventUpdate.content['event_id']); - if (i < events.length) { - removeAggregatedEvent(events[i]); - events.removeAt(i); - onRemove?.call(i); + final i = _findEvent( + event_id: eventUpdate.content['event_id'], + unsigned_txid: eventUpdate.content['unsigned'] is Map + ? eventUpdate.content['unsigned']['transaction_id'] + : null); + + if (i < events.length) { + // if the old status is larger than the new one, we also want to preserve the old status + final oldStatus = events[i].status; + events[i] = Event.fromJson( + eventUpdate.content, + room, + ); + // do we preserve the status? we should allow 0 -> -1 updates and status increases + if ((latestEventStatus(status, oldStatus) == oldStatus) && + !(status.isError && oldStatus.isSending)) { + events[i].status = oldStatus; } + addAggregatedEvent(events[i]); + onChange?.call(i); } else { - final i = _findEvent( - event_id: eventUpdate.content['event_id'], - unsigned_txid: eventUpdate.content['unsigned'] is Map - ? eventUpdate.content['unsigned']['transaction_id'] - : null); + final newEvent = Event.fromJson( + eventUpdate.content, + room, + ); - if (i < events.length) { - // if the old status is larger than the new one, we also want to preserve the old status - final oldStatus = events[i].status; - events[i] = Event.fromJson( - eventUpdate.content, - room, - ); - // do we preserve the status? we should allow 0 -> -1 updates and status increases - if ((latestEventStatus(status, oldStatus) == oldStatus) && - !(status.isError && oldStatus.isSending)) { - events[i].status = oldStatus; - } - addAggregatedEvent(events[i]); - onChange?.call(i); + if (eventUpdate.type == EventUpdateType.history && + events.indexWhere( + (e) => e.eventId == eventUpdate.content['event_id']) != + -1) return; + var index = events.length; + if (eventUpdate.type == EventUpdateType.history) { + events.add(newEvent); } else { - final newEvent = Event.fromJson( - eventUpdate.content, - room, - ); - - if (eventUpdate.type == EventUpdateType.history && - events.indexWhere( - (e) => e.eventId == eventUpdate.content['event_id']) != - -1) return; - var index = events.length; - if (eventUpdate.type == EventUpdateType.history) { - events.add(newEvent); - } else { - index = events.firstIndexWhereNotError; - events.insert(index, newEvent); - } - onInsert?.call(index); - - addAggregatedEvent(newEvent); + index = events.firstIndexWhereNotError; + events.insert(index, newEvent); } + onInsert?.call(index); + + addAggregatedEvent(newEvent); } // Handle redaction events diff --git a/test/event_test.dart b/test/event_test.dart index 56da26d9..8428b078 100644 --- a/test/event_test.dart +++ b/test/event_test.dart @@ -242,12 +242,12 @@ void main() { test('remove', () async { final event = Event.fromJson( - jsonObj, Room(id: '1234', client: Client('testclient'))); - final removed1 = await event.remove(); + jsonObj, + Room(id: '1234', client: Client('testclient')), + ); + expect(() async => await event.cancelSend(), throwsException); event.status = EventStatus.sending; - final removed2 = await event.remove(); - expect(removed1, false); - expect(removed2, true); + await event.cancelSend(); }); test('sendAgain', () async { diff --git a/test/timeline_context_test.dart b/test/timeline_context_test.dart index 6b306801..1c2fbb2d 100644 --- a/test/timeline_context_test.dart +++ b/test/timeline_context_test.dart @@ -270,7 +270,7 @@ void main() { )); await waitForCount(7); - await timeline.events[0].remove(); + await timeline.events[0].cancelSend(); await waitForCount(8); expect(updateCount, 8); diff --git a/test/timeline_test.dart b/test/timeline_test.dart index 54c87164..f82e511f 100644 --- a/test/timeline_test.dart +++ b/test/timeline_test.dart @@ -507,7 +507,7 @@ void main() { )); await waitForCount(1); - await timeline.events[0].remove(); + await timeline.events[0].cancelSend(); await waitForCount(2);