refactor: Make RoomUpdate class null safe by removing it

RoomUpdate came from a time where we had no data model for
SyncUpdates but now we have and therefore this class is just
code duplication. This removes the class
and uses the SyncRoomUpdate class from
the package matrix_api_lite instead.
It needed a lot of refactoring at some places
where I also have removed some unnecessary null or type checks.
This commit is contained in:
Christian Pauly 2021-09-08 09:54:39 +02:00 committed by Krille Fear
parent 5b13e0442e
commit e13b00d127
9 changed files with 112 additions and 212 deletions

View File

@ -21,7 +21,6 @@
library matrix; library matrix;
export 'package:matrix_api_lite/matrix_api_lite.dart'; export 'package:matrix_api_lite/matrix_api_lite.dart';
export 'src/utils/room_update.dart';
export 'src/utils/event_update.dart'; export 'src/utils/event_update.dart';
export 'src/utils/image_pack_extension.dart'; export 'src/utils/image_pack_extension.dart';
export 'src/utils/device_keys_list.dart'; export 'src/utils/device_keys_list.dart';

View File

@ -37,7 +37,6 @@ import 'utils/device_keys_list.dart';
import 'utils/event_update.dart'; import 'utils/event_update.dart';
import 'utils/http_timeout.dart'; import 'utils/http_timeout.dart';
import 'utils/matrix_file.dart'; import 'utils/matrix_file.dart';
import 'utils/room_update.dart';
import 'utils/to_device_event.dart'; import 'utils/to_device_event.dart';
import 'utils/uia_request.dart'; import 'utils/uia_request.dart';
import 'utils/multilock.dart'; import 'utils/multilock.dart';
@ -758,11 +757,6 @@ class Client extends MatrixApi {
/// onRoomEvent( "m.room.message", "!chat_id:server.com", "timeline", {sender: "@bob:server.com", body: "Hello world"} ) /// onRoomEvent( "m.room.message", "!chat_id:server.com", "timeline", {sender: "@bob:server.com", body: "Hello world"} )
final StreamController<EventUpdate> onEvent = StreamController.broadcast(); final StreamController<EventUpdate> onEvent = StreamController.broadcast();
/// Outside of the events there are updates for the global chat states which
/// are handled by this signal:
final StreamController<RoomUpdate> onRoomUpdate =
StreamController.broadcast();
/// The onToDeviceEvent is called when there comes a new to device event. It is /// The onToDeviceEvent is called when there comes a new to device event. It is
/// already decrypted if necessary. /// already decrypted if necessary.
final StreamController<ToDeviceEvent> onToDeviceEvent = final StreamController<ToDeviceEvent> onToDeviceEvent =
@ -1255,14 +1249,12 @@ class Client extends MatrixApi {
final id = entry.key; final id = entry.key;
final room = entry.value; final room = entry.value;
final update = RoomUpdate.fromSyncRoomUpdate(room, id);
if (database != null) { if (database != null) {
// TODO: This method seems to be rather slow for some updates // TODO: This method seems to be rather slow for some updates
// Perhaps don't dynamically build that one query? // Perhaps don't dynamically build that one query?
await database.storeRoomUpdate(this.id, update, getRoomById(id)); await database.storeRoomUpdate(this.id, id, room, getRoomById(id));
} }
_updateRoomsByRoomUpdate(update); _updateRoomsByRoomUpdate(id, room);
onRoomUpdate.add(update);
/// Handle now all room events and save them in the database /// Handle now all room events and save them in the database
if (room is JoinedRoomUpdate) { if (room is JoinedRoomUpdate) {
@ -1432,49 +1424,58 @@ class Client extends MatrixApi {
} }
} }
void _updateRoomsByRoomUpdate(RoomUpdate chatUpdate) { void _updateRoomsByRoomUpdate(String roomId, SyncRoomUpdate chatUpdate) {
// Update the chat list item. // Update the chat list item.
// Search the room in the rooms // Search the room in the rooms
num j = 0; num j = 0;
for (j = 0; j < rooms.length; j++) { for (j = 0; j < rooms.length; j++) {
if (rooms[j].id == chatUpdate.id) break; if (rooms[j].id == roomId) break;
} }
final found = (j < rooms.length && rooms[j].id == chatUpdate.id); final found = (j < rooms.length && rooms[j].id == roomId);
final isLeftRoom = chatUpdate.membership == Membership.leave; final membership = chatUpdate is LeftRoomUpdate
? Membership.leave
: chatUpdate is InvitedRoomUpdate
? Membership.invite
: Membership.join;
// Does the chat already exist in the list rooms? // Does the chat already exist in the list rooms?
if (!found && !isLeftRoom) { if (!found && membership != Membership.leave) {
final position = chatUpdate.membership == Membership.invite ? 0 : j; final position = membership == Membership.invite ? 0 : j;
// Add the new chat to the list // Add the new chat to the list
final newRoom = Room( final newRoom = chatUpdate is JoinedRoomUpdate
id: chatUpdate.id, ? Room(
membership: chatUpdate.membership, id: roomId,
prev_batch: chatUpdate.prev_batch, membership: membership,
highlightCount: chatUpdate.highlight_count, prev_batch: chatUpdate.timeline?.prevBatch,
notificationCount: chatUpdate.notification_count, highlightCount: chatUpdate.unreadNotifications?.highlightCount,
summary: chatUpdate.summary, notificationCount:
roomAccountData: {}, chatUpdate.unreadNotifications?.notificationCount,
client: this, summary: chatUpdate.summary,
); client: this,
)
: Room(id: roomId, membership: membership, client: this);
rooms.insert(position, newRoom); rooms.insert(position, newRoom);
} }
// If the membership is "leave" then remove the item and stop here // If the membership is "leave" then remove the item and stop here
else if (found && isLeftRoom) { else if (found && membership == Membership.leave) {
rooms.removeAt(j); rooms.removeAt(j);
} }
// Update notification, highlight count and/or additional informations // Update notification, highlight count and/or additional informations
else if (found && else if (found &&
chatUpdate.membership != Membership.leave && chatUpdate is JoinedRoomUpdate &&
(rooms[j].membership != chatUpdate.membership || (rooms[j].membership != membership ||
rooms[j].notificationCount != chatUpdate.notification_count || rooms[j].notificationCount !=
rooms[j].highlightCount != chatUpdate.highlight_count || (chatUpdate.unreadNotifications?.notificationCount ?? 0) ||
rooms[j].highlightCount !=
(chatUpdate.unreadNotifications?.highlightCount ?? 0) ||
chatUpdate.summary != null || chatUpdate.summary != null ||
chatUpdate.prev_batch != null)) { chatUpdate.timeline?.prevBatch != null)) {
rooms[j].membership = chatUpdate.membership; rooms[j].membership = membership;
rooms[j].notificationCount = chatUpdate.notification_count; rooms[j].notificationCount =
rooms[j].highlightCount = chatUpdate.highlight_count; chatUpdate.unreadNotifications?.notificationCount;
if (chatUpdate.prev_batch != null) { rooms[j].highlightCount = chatUpdate.unreadNotifications?.highlightCount;
rooms[j].prev_batch = chatUpdate.prev_batch; if (chatUpdate.timeline?.prevBatch != null) {
rooms[j].prev_batch = chatUpdate.timeline?.prevBatch;
} }
if (chatUpdate.summary != null) { if (chatUpdate.summary != null) {
final roomSummaryJson = rooms[j].summary.toJson() final roomSummaryJson = rooms[j].summary.toJson()
@ -1482,7 +1483,8 @@ class Client extends MatrixApi {
rooms[j].summary = RoomSummary.fromJson(roomSummaryJson); rooms[j].summary = RoomSummary.fromJson(roomSummaryJson);
} }
if (rooms[j].onUpdate != null) rooms[j].onUpdate.add(rooms[j].id); if (rooms[j].onUpdate != null) rooms[j].onUpdate.add(rooms[j].id);
if (chatUpdate.limitedTimeline && requestHistoryOnLimitedTimeline) { if ((chatUpdate?.timeline?.limited ?? false) &&
requestHistoryOnLimitedTimeline) {
Logs().v('Limited timeline for ${rooms[j].id} request history now'); Logs().v('Limited timeline for ${rooms[j].id} request history now');
runInRoot(rooms[j].requestHistory); runInRoot(rooms[j].requestHistory);
} }

View File

@ -59,7 +59,8 @@ abstract class DatabaseApi {
/// Stores a RoomUpdate object in the database. Must be called inside of /// Stores a RoomUpdate object in the database. Must be called inside of
/// [transaction]. /// [transaction].
Future<void> storeRoomUpdate(int clientId, RoomUpdate roomUpdate, Future<void> storeRoomUpdate(
int clientId, String roomId, SyncRoomUpdate roomUpdate,
[Room oldRoom]); [Room oldRoom]);
/// Stores an EventUpdate object in the database. Must be called inside of /// Stores an EventUpdate object in the database. Must be called inside of

View File

@ -990,38 +990,55 @@ class FamedlySdkHiveDatabase extends DatabaseApi {
} }
@override @override
Future<void> storeRoomUpdate(int clientId, RoomUpdate roomUpdate, Future<void> storeRoomUpdate(
int clientId, String roomId, SyncRoomUpdate roomUpdate,
[dynamic _]) async { [dynamic _]) async {
// Leave room if membership is leave // Leave room if membership is leave
if ({Membership.leave, Membership.ban}.contains(roomUpdate.membership)) { if (roomUpdate is LeftRoomUpdate) {
await forgetRoom(clientId, roomUpdate.id); await forgetRoom(clientId, roomId);
return; return;
} }
final membership = roomUpdate is LeftRoomUpdate
? Membership.leave
: roomUpdate is InvitedRoomUpdate
? Membership.invite
: Membership.join;
// Make sure room exists // Make sure room exists
if (!_roomsBox.containsKey(roomUpdate.id.toHiveKey)) { if (!_roomsBox.containsKey(roomId.toHiveKey)) {
await _roomsBox.put( await _roomsBox.put(
roomUpdate.id.toHiveKey, roomId.toHiveKey,
Room( roomUpdate is JoinedRoomUpdate
id: roomUpdate.id, ? Room(
membership: roomUpdate.membership, id: roomId,
highlightCount: roomUpdate.highlight_count?.toInt(), membership: membership,
notificationCount: roomUpdate.notification_count?.toInt(), highlightCount:
prev_batch: roomUpdate.prev_batch, roomUpdate.unreadNotifications?.highlightCount?.toInt(),
summary: roomUpdate.summary, notificationCount: roomUpdate
).toJson()); .unreadNotifications?.notificationCount
} else { ?.toInt(),
final currentRawRoom = await _roomsBox.get(roomUpdate.id.toHiveKey); prev_batch: roomUpdate.timeline?.prevBatch,
summary: roomUpdate.summary,
).toJson()
: Room(
id: roomId,
membership: membership,
).toJson());
} else if (roomUpdate is JoinedRoomUpdate) {
final currentRawRoom = await _roomsBox.get(roomId.toHiveKey);
final currentRoom = Room.fromJson(convertToJson(currentRawRoom)); final currentRoom = Room.fromJson(convertToJson(currentRawRoom));
await _roomsBox.put( await _roomsBox.put(
roomUpdate.id.toHiveKey, roomId.toHiveKey,
Room( Room(
id: roomUpdate.id, id: roomId,
membership: roomUpdate.membership ?? currentRoom.membership, membership: membership,
highlightCount: roomUpdate.highlight_count?.toInt() ?? highlightCount:
currentRoom.highlightCount, roomUpdate.unreadNotifications?.highlightCount?.toInt() ??
notificationCount: roomUpdate.notification_count?.toInt() ?? currentRoom.highlightCount,
currentRoom.notificationCount, notificationCount:
prev_batch: roomUpdate.prev_batch ?? currentRoom.prev_batch, roomUpdate.unreadNotifications?.notificationCount?.toInt() ??
currentRoom.notificationCount,
prev_batch:
roomUpdate.timeline?.prevBatch ?? currentRoom.prev_batch,
summary: RoomSummary.fromJson(currentRoom.summary.toJson() summary: RoomSummary.fromJson(currentRoom.summary.toJson()
..addAll(roomUpdate.summary?.toJson() ?? {})), ..addAll(roomUpdate.summary?.toJson() ?? {})),
).toJson()); ).toJson());
@ -1029,9 +1046,9 @@ class FamedlySdkHiveDatabase extends DatabaseApi {
// Is the timeline limited? Then all previous messages should be // Is the timeline limited? Then all previous messages should be
// removed from the database! // removed from the database!
if (roomUpdate.limitedTimeline) { if (roomUpdate is JoinedRoomUpdate &&
await _timelineFragmentsBox roomUpdate.timeline?.limited == true) {
.delete(MultiKey(roomUpdate.id, '').toString()); await _timelineFragmentsBox.delete(MultiKey(roomId, '').toString());
} }
} }

View File

@ -23,7 +23,6 @@ import '../matrix.dart';
import 'event.dart'; import 'event.dart';
import 'room.dart'; import 'room.dart';
import 'utils/event_update.dart'; import 'utils/event_update.dart';
import 'utils/room_update.dart';
/// Represents the timeline of a room. The callback [onUpdate] will be triggered /// Represents the timeline of a room. The callback [onUpdate] will be triggered
/// automatically. The initial /// automatically. The initial
@ -39,7 +38,7 @@ class Timeline {
final void Function(int insertID) onInsert; final void Function(int insertID) onInsert;
StreamSubscription<EventUpdate> sub; StreamSubscription<EventUpdate> sub;
StreamSubscription<RoomUpdate> roomSub; StreamSubscription<SyncUpdate> roomSub;
StreamSubscription<String> sessionIdReceivedSub; StreamSubscription<String> sessionIdReceivedSub;
bool isRequestingHistory = false; bool isRequestingHistory = false;
@ -107,13 +106,13 @@ class Timeline {
Timeline({this.room, List<Event> events, this.onUpdate, this.onInsert}) Timeline({this.room, List<Event> events, this.onUpdate, this.onInsert})
: events = events ?? [] { : events = events ?? [] {
sub ??= room.client.onEvent.stream.listen(_handleEventUpdate); sub ??= room.client.onEvent.stream.listen(_handleEventUpdate);
// if the timeline is limited we want to clear our events cache // If the timeline is limited we want to clear our events cache
// as r.limitedTimeline can be "null" sometimes, we need to check for == true roomSub ??= room.client.onSync.stream
// as after receiving a limited timeline room update new events are expected .where((sync) =>
// to be received via the onEvent stream, it is unneeded to call sortAndUpdate sync.rooms?.join != null &&
roomSub ??= room.client.onRoomUpdate.stream sync.rooms.join.containsKey(room.id) &&
.where((r) => r.id == room.id && r.limitedTimeline == true) sync.rooms.join[room.id]?.timeline?.limited == true)
.listen((r) { .listen((_) {
events.clear(); events.clear();
aggregatedEvents.clear(); aggregatedEvents.clear();
}); });

View File

@ -1,93 +0,0 @@
// @dart=2.9
/*
* Famedly Matrix SDK
* Copyright (C) 2019, 2020, 2021 Famedly GmbH
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
import '../../matrix.dart';
/// Represents a new room or an update for an
/// already known room.
class RoomUpdate {
/// All rooms have an idea in the format: !uniqueid:server.abc
final String id;
/// The current membership state of the user in this room.
final Membership membership;
/// Represents the number of unead notifications. This probably doesn't fit the number
/// of unread messages.
final num notification_count;
// The number of unread highlighted notifications.
final num highlight_count;
/// If there are too much new messages, the `homeserver` will only send the
/// last X (default is 10) messages and set the `limitedTimeline` flag to true.
final bool limitedTimeline;
/// Represents the current position of the client in the room history.
final String prev_batch;
final RoomSummary summary;
RoomUpdate({
this.id,
this.membership,
this.notification_count,
this.highlight_count,
this.limitedTimeline,
this.prev_batch,
this.summary,
});
factory RoomUpdate.fromSyncRoomUpdate(
SyncRoomUpdate update,
String roomId,
) =>
update is JoinedRoomUpdate
? RoomUpdate(
id: roomId,
membership: Membership.join,
notification_count:
update.unreadNotifications?.notificationCount ?? 0,
highlight_count: update.unreadNotifications?.highlightCount ?? 0,
limitedTimeline: update.timeline?.limited ?? false,
prev_batch: update.timeline?.prevBatch,
summary: update.summary,
)
: update is InvitedRoomUpdate
? RoomUpdate(
id: roomId,
membership: Membership.invite,
notification_count: 0,
highlight_count: 0,
limitedTimeline: false,
prev_batch: null,
summary: null,
)
: update is LeftRoomUpdate
? RoomUpdate(
id: roomId,
membership: Membership.leave,
notification_count: 0,
highlight_count: 0,
limitedTimeline: update.timeline?.limited ?? false,
prev_batch: update.timeline?.prevBatch,
summary: null,
)
: null;
}

View File

@ -26,7 +26,6 @@ import 'package:matrix/matrix.dart';
import 'package:matrix/src/client.dart'; import 'package:matrix/src/client.dart';
import 'package:matrix/src/utils/event_update.dart'; import 'package:matrix/src/utils/event_update.dart';
import 'package:matrix/src/utils/matrix_file.dart'; import 'package:matrix/src/utils/matrix_file.dart';
import 'package:matrix/src/utils/room_update.dart';
import 'package:olm/olm.dart' as olm; import 'package:olm/olm.dart' as olm;
import 'package:test/test.dart'; import 'package:test/test.dart';
import 'package:canonical_json/canonical_json.dart'; import 'package:canonical_json/canonical_json.dart';
@ -38,7 +37,6 @@ import 'fake_matrix_api.dart';
void main() { void main() {
Client matrix; Client matrix;
Future<List<RoomUpdate>> roomUpdateListFuture;
Future<List<EventUpdate>> eventUpdateListFuture; Future<List<EventUpdate>> eventUpdateListFuture;
Future<List<ToDeviceEvent>> toDeviceUpdateListFuture; Future<List<ToDeviceEvent>> toDeviceUpdateListFuture;
@ -56,7 +54,6 @@ void main() {
matrix = Client('testclient', httpClient: FakeMatrixApi()); matrix = Client('testclient', httpClient: FakeMatrixApi());
roomUpdateListFuture = matrix.onRoomUpdate.stream.toList();
eventUpdateListFuture = matrix.onEvent.stream.toList(); eventUpdateListFuture = matrix.onEvent.stream.toList();
toDeviceUpdateListFuture = matrix.onToDeviceEvent.stream.toList(); toDeviceUpdateListFuture = matrix.onToDeviceEvent.stream.toList();
@ -217,28 +214,6 @@ void main() {
expect(loginState, LoginState.loggedOut); expect(loginState, LoginState.loggedOut);
}); });
test('Room Update Test', () async {
await matrix.onRoomUpdate.close();
final roomUpdateList = await roomUpdateListFuture;
expect(roomUpdateList.length, 4);
expect(roomUpdateList[0].id == '!726s6s6q:example.com', true);
expect(roomUpdateList[0].membership == Membership.join, true);
expect(roomUpdateList[0].prev_batch == 't34-23535_0_0', true);
expect(roomUpdateList[0].limitedTimeline == true, true);
expect(roomUpdateList[0].notification_count == 2, true);
expect(roomUpdateList[0].highlight_count == 2, true);
expect(roomUpdateList[1].id == '!696r7674:example.com', true);
expect(roomUpdateList[1].membership == Membership.invite, true);
expect(roomUpdateList[1].prev_batch == null, true);
expect(roomUpdateList[1].limitedTimeline == false, true);
expect(roomUpdateList[1].notification_count == 0, true);
expect(roomUpdateList[1].highlight_count == 0, true);
});
test('Event Update Test', () async { test('Event Update Test', () async {
await matrix.onEvent.close(); await matrix.onEvent.close();
@ -318,7 +293,6 @@ void main() {
test('Login', () async { test('Login', () async {
matrix = Client('testclient', httpClient: FakeMatrixApi()); matrix = Client('testclient', httpClient: FakeMatrixApi());
roomUpdateListFuture = matrix.onRoomUpdate.stream.toList();
eventUpdateListFuture = matrix.onEvent.stream.toList(); eventUpdateListFuture = matrix.onEvent.stream.toList();
await matrix.checkHomeserver('https://fakeserver.notexisting', await matrix.checkHomeserver('https://fakeserver.notexisting',

View File

@ -106,15 +106,13 @@ void testDatabase(Future<DatabaseApi> futureDatabase, int clientId) {
expect(file == null, true); expect(file == null, true);
}); });
test('storeRoomUpdate', () async { test('storeRoomUpdate', () async {
await database.storeRoomUpdate( final roomUpdate = JoinedRoomUpdate.fromJson({
clientId, 'highlight_count': 0,
RoomUpdate( 'notification_count': 0,
id: '!testroom', 'limited_timeline': false,
highlight_count: 0, 'membership': Membership.join,
notification_count: 0, });
limitedTimeline: false, await database.storeRoomUpdate(clientId, '!testroom', roomUpdate);
membership: Membership.join,
));
final rooms = await database.getRoomList(Client('testclient')); final rooms = await database.getRoomList(Client('testclient'));
expect(rooms.single.id, '!testroom'); expect(rooms.single.id, '!testroom');
}); });

View File

@ -24,7 +24,6 @@ import 'package:matrix/src/client.dart';
import 'package:matrix/src/room.dart'; import 'package:matrix/src/room.dart';
import 'package:matrix/src/timeline.dart'; import 'package:matrix/src/timeline.dart';
import 'package:matrix/src/utils/event_update.dart'; import 'package:matrix/src/utils/event_update.dart';
import 'package:matrix/src/utils/room_update.dart';
import 'package:olm/olm.dart' as olm; import 'package:olm/olm.dart' as olm;
import 'fake_client.dart'; import 'fake_client.dart';
@ -293,14 +292,18 @@ void main() {
}); });
test('Clear cache on limited timeline', () async { test('Clear cache on limited timeline', () async {
client.onRoomUpdate.add(RoomUpdate( client.onSync.add(SyncUpdate(nextBatch: '1234')
id: roomID, ..rooms = (RoomsUpdate()
membership: Membership.join, ..join = {
notification_count: 0, roomID: (JoinedRoomUpdate()
highlight_count: 0, ..timeline = (TimelineUpdate()
limitedTimeline: true, ..limited = true
prev_batch: 'blah', ..prevBatch = 'blah')
)); ..unreadNotifications = UnreadNotificationCounts.fromJson({
'highlight_count': 0,
'notification_count': 0,
}))
}));
await Future.delayed(Duration(milliseconds: 50)); await Future.delayed(Duration(milliseconds: 50));
expect(timeline.events.isEmpty, true); expect(timeline.events.isEmpty, true);
}); });