diff --git a/lib/fake_matrix_api.dart b/lib/fake_matrix_api.dart index ae4c63b3..a1df999e 100644 --- a/lib/fake_matrix_api.dart +++ b/lib/fake_matrix_api.dart @@ -168,8 +168,9 @@ class FakeMatrixApi extends BaseClient { } else if (method == 'GET' && action.contains('/client/v3/rooms/') && action.contains('/state/m.room.member/') && - !action.endsWith('%40alicyy%3Aexample.com')) { - res = {'displayname': ''}; + !action.endsWith('%40alicyy%3Aexample.com') && + !action.contains('%40getme')) { + res = {'displayname': '', 'membership': 'ban'}; } else if (method == 'PUT' && action.contains( '/client/v3/rooms/!1234%3AfakeServer.notExisting/send/')) { @@ -1522,15 +1523,28 @@ class FakeMatrixApi extends BaseClient { {'visibility': 'public'}, '/client/v3/rooms/1/state/m.room.member/@alice:example.com': (var req) => {'displayname': 'Alice'}, + '/client/v3/profile/%40getmeprofile%3Aexample.com': (var req) => { + 'avatar_url': 'mxc://test', + 'displayname': 'You got me (profile)', + }, '/client/v3/profile/%40getme%3Aexample.com': (var req) => { 'avatar_url': 'mxc://test', 'displayname': 'You got me', }, - '/client/v3/rooms/!localpart%3Aserver.abc/state/m.room.member/@getme%3Aexample.com': + '/client/v3/rooms/!localpart%3Aserver.abc/state/m.room.member/%40getme%3Aexample.com': (var req) => { 'avatar_url': 'mxc://test', 'displayname': 'You got me', + 'membership': 'knock', }, + '/client/v3/rooms/!localpart%3Aserver.abc/state/m.room.member/%40getmeempty%3Aexample.com': + (var req) => { + 'membership': 'leave', + }, + '/client/v3/profile/%40getmeempty%3Aexample.com': (var req) => { + 'avatar_url': 'mxc://test', + 'displayname': 'You got me (empty)', + }, '/client/v3/rooms/!localpart%3Aserver.abc/state': (var req) => [ { 'content': {'join_rule': 'public'}, @@ -1596,11 +1610,6 @@ class FakeMatrixApi extends BaseClient { 'state_key': '' } ], - '/client/v3/rooms/!localpart:server.abc/state/m.room.member/@getme:example.com': - (var req) => { - 'avatar_url': 'mxc://test', - 'displayname': 'You got me', - }, '/client/v3/rooms/!localpart:server.abc/event/1234': (var req) => { 'content': { 'body': 'This is an example text message', diff --git a/lib/src/room.dart b/lib/src/room.dart index 3302cd00..b503dfd3 100644 --- a/lib/src/room.dart +++ b/lib/src/room.dart @@ -18,7 +18,9 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:math'; +import 'package:async/async.dart'; import 'package:collection/collection.dart'; import 'package:html_unescape/html_unescape.dart'; @@ -1648,100 +1650,169 @@ class Room { } } - final Set _requestingMatrixIds = {}; + // Internal helper to implement requestUser + Future _requestSingleParticipantViaState( + String mxID, { + required bool ignoreErrors, + }) async { + try { + Logs().v( + 'Request missing user $mxID in room ${getLocalizedDisplayname()} from the server...', + ); + final resp = await client.getRoomStateWithKey( + id, + EventTypes.RoomMember, + mxID, + ); + + // valid member events require a valid membership key + final membership = resp.tryGet('membership', TryGet.required); + assert(membership != null); + + final foundUser = User( + mxID, + room: this, + displayName: resp.tryGet('displayname', TryGet.silent), + avatarUrl: resp.tryGet('avatar_url', TryGet.silent), + membership: membership, + ); + + // Store user in database: + await client.database?.transaction(() async { + await client.database?.storeEventUpdate( + EventUpdate( + content: foundUser.toJson(), + roomID: id, + type: EventUpdateType.state, + ), + client, + ); + }); + + return foundUser; + } on MatrixException catch (_) { + // Ignore if we have no permission + return null; + } catch (e, s) { + if (!ignoreErrors) { + rethrow; + } else { + Logs().w('Unable to request the user $mxID from the server', e, s); + return null; + } + } + } + + // Internal helper to implement requestUser + Future _requestUser( + String mxID, { + required bool ignoreErrors, + required bool requestState, + required bool requestProfile, + }) async { + // Is user already in cache? + final userFromState = getState(EventTypes.RoomMember, mxID)?.asUser(this); + + // If not in cache, try the database + var foundUser = userFromState; + + // If the room is not postloaded, check the database + if (partial && foundUser == null) { + foundUser = await client.database?.getUser(mxID, this); + } + + // If not in the database, try fetching the member from the server + if (requestState && foundUser == null) { + foundUser = await _requestSingleParticipantViaState(mxID, + ignoreErrors: ignoreErrors); + } + + // If the user isn't found or they have left and no displayname set anymore, request their profile from the server + if (requestProfile) { + if (foundUser + case null || + User( + membership: Membership.ban || Membership.leave, + displayName: null + )) { + try { + final profile = await client.getProfileFromUserId(mxID); + foundUser = User( + mxID, + displayName: profile.displayName, + avatarUrl: profile.avatarUrl?.toString(), + membership: foundUser?.membership.name ?? Membership.leave.name, + room: this, + ); + } catch (e, s) { + if (!ignoreErrors) { + rethrow; + } else { + Logs() + .w('Unable to request the profile $mxID from the server', e, s); + } + } + } + } + + if (foundUser == null) return null; + + // Set user in the local state if the state changed. + // If we set the state unconditionally, we might end up with a client calling this over and over thinking the user changed. + if (userFromState == null || + userFromState.displayName != foundUser.displayName) { + setState(foundUser); + // ignore: deprecated_member_use_from_same_package + onUpdate.add(id); + } + + return foundUser; + } + + final Map< + ({ + String mxID, + bool ignoreErrors, + bool requestState, + bool requestProfile, + }), + AsyncCache> _inflightUserRequests = {}; /// Requests a missing [User] for this room. Important for clients using /// lazy loading. If the user can't be found this method tries to fetch - /// the displayname and avatar from the profile if [requestProfile] is true. + /// the displayname and avatar from the server if [requestState] is true. + /// If that fails, it falls back to requesting the global profile if + /// [requestProfile] is true. Future requestUser( String mxID, { bool ignoreErrors = false, + bool requestState = true, bool requestProfile = true, }) async { assert(mxID.isValidMatrixId); - // Is user already in cache? - var foundUser = getState(EventTypes.RoomMember, mxID)?.asUser(this); + final parameters = ( + mxID: mxID, + ignoreErrors: ignoreErrors, + requestState: requestState, + requestProfile: requestProfile, + ); - // If not, is it in the database? - foundUser ??= await client.database?.getUser(mxID, this); + final cache = _inflightUserRequests[parameters] ??= AsyncCache.ephemeral(); - // If not, can we request it from the server? - if (foundUser == null) { - if (!_requestingMatrixIds.add(mxID)) return null; - Map? resp; - try { - Logs().v( - 'Request missing user $mxID in room ${getLocalizedDisplayname()} from the server...'); - resp = await client.getRoomStateWithKey( - id, - EventTypes.RoomMember, - mxID, - ); - foundUser = User( - mxID, - room: this, - displayName: resp['displayname'], - avatarUrl: resp['avatar_url'], - membership: resp['membership'], - ); - _requestingMatrixIds.remove(mxID); - - // Store user in database: - await client.database?.transaction(() async { - await client.database?.storeEventUpdate( - EventUpdate( - content: foundUser!.toJson(), - roomID: id, - type: EventUpdateType.state, - ), - client, - ); - }); - } on MatrixException catch (_) { - // Ignore if we have no permission - } catch (e, s) { - if (!ignoreErrors) { - _requestingMatrixIds.remove(mxID); - rethrow; - } else { - Logs().w('Unable to request the user $mxID from the server', e, s); - } - } + try { + final user = await cache.fetch(() => _requestUser( + mxID, + ignoreErrors: ignoreErrors, + requestState: requestState, + requestProfile: requestProfile, + )); + _inflightUserRequests.remove(parameters); + return user; + } catch (_) { + _inflightUserRequests.remove(parameters); + rethrow; } - - // User not found anywhere? Set a blank one: - foundUser ??= User(mxID, room: this, membership: 'leave'); - - // Is it a left user without any displayname/avatar info? Try fetch profile: - if (requestProfile && - {Membership.ban, Membership.leave}.contains(foundUser.membership) && - foundUser.displayName == null && - foundUser.avatarUrl == null) { - try { - final profile = await client.getProfileFromUserId(mxID); - foundUser = User( - mxID, - displayName: profile.displayName, - avatarUrl: profile.avatarUrl?.toString(), - membership: Membership.leave.name, - room: this, - ); - } catch (e, s) { - if (!ignoreErrors) { - rethrow; - } else { - Logs().w('Unable to request the profile $mxID from the server', e, s); - } - } - } - - // Set user in the local state - setState(foundUser!); - // ignore: deprecated_member_use_from_same_package - onUpdate.add(id); - - return foundUser; } /// Searches for the event in the local cache and then on the server if not @@ -2272,7 +2343,7 @@ class Room { 'https://matrix.to/#/${Uri.encodeComponent(canonicalAlias)}'); } final List queryParameters = []; - final users = await requestParticipants(); + final users = await requestParticipants([Membership.join]); final currentPowerLevelsMap = getState(EventTypes.RoomPowerLevels)?.content; final temp = List.from(users); @@ -2301,18 +2372,17 @@ class Room { } } final sortedServers = Map.fromEntries(servers.entries.toList() - ..sort((e1, e2) => e1.value.compareTo(e2.value))); - for (var i = 0; i <= 2; i++) { - if (!queryParameters.contains(sortedServers.keys.last)) { - queryParameters.add(sortedServers.keys.last); + ..sort((e1, e2) => e2.value.compareTo(e1.value))) + .keys + .take(3); + for (final server in sortedServers) { + if (!queryParameters.contains(server)) { + queryParameters.add(server); } - sortedServers.remove(sortedServers.keys.last); } var queryString = '?'; - for (var i = 0; - i <= (queryParameters.length > 2 ? 2 : queryParameters.length); - i++) { + for (var i = 0; i < min(queryParameters.length, 3); i++) { if (i != 0) { queryString += '&'; } diff --git a/test/room_test.dart b/test/room_test.dart index 8b4deb2c..a6935cee 100644 --- a/test/room_test.dart +++ b/test/room_test.dart @@ -682,12 +682,83 @@ void main() { }); test('getUserByMXID', () async { - User? user; - try { - user = await room.requestUser('@getme:example.com'); - } catch (_) {} + final List called = []; + final List called2 = []; + // ignore: deprecated_member_use_from_same_package + final subscription = room.onUpdate.stream.listen((i) { + called.add(i); + }); + final subscription2 = room.client.onRoomState.stream.listen((i) { + called2.add(i.roomId); + }); + + FakeMatrixApi.calledEndpoints.clear(); + final user = await room.requestUser('@getme:example.com'); + expect(FakeMatrixApi.calledEndpoints.keys, [ + '/client/v3/rooms/!localpart%3Aserver.abc/state/m.room.member/%40getme%3Aexample.com' + ]); expect(user?.stateKey, '@getme:example.com'); - expect(user?.calcDisplayname(), 'Getme'); + expect(user?.calcDisplayname(), 'You got me'); + expect(user?.membership, Membership.knock); + + // Yield for the onUpdate + await Future.delayed(Duration( + milliseconds: 1, + )); + expect(called.length, 1); + expect(called2.length, 1); + + FakeMatrixApi.calledEndpoints.clear(); + final user2 = await room.requestUser('@getmeprofile:example.com'); + expect(FakeMatrixApi.calledEndpoints.keys, [ + '/client/v3/rooms/!localpart%3Aserver.abc/state/m.room.member/%40getmeprofile%3Aexample.com', + '/client/v3/profile/%40getmeprofile%3Aexample.com' + ]); + expect(user2?.stateKey, '@getmeprofile:example.com'); + expect(user2?.calcDisplayname(), 'You got me (profile)'); + expect(user2?.membership, Membership.leave); + + // Yield for the onUpdate + await Future.delayed(Duration( + milliseconds: 1, + )); + expect(called.length, 2); + expect(called2.length, 2); + + FakeMatrixApi.calledEndpoints.clear(); + final userAgain = await room.requestUser('@getme:example.com'); + expect(FakeMatrixApi.calledEndpoints.keys, []); + expect(userAgain?.stateKey, '@getme:example.com'); + expect(userAgain?.calcDisplayname(), 'You got me'); + expect(userAgain?.membership, Membership.knock); + + // Yield for the onUpdate + await Future.delayed(Duration( + milliseconds: 1, + )); + expect(called.length, 2, reason: 'onUpdate should not have been called.'); + expect(called2.length, 2, + reason: 'onRoomState should not have been called.'); + + FakeMatrixApi.calledEndpoints.clear(); + final user3 = await room.requestUser('@getmeempty:example.com'); + expect(FakeMatrixApi.calledEndpoints.keys, [ + '/client/v3/rooms/!localpart%3Aserver.abc/state/m.room.member/%40getmeempty%3Aexample.com', + '/client/v3/profile/%40getmeempty%3Aexample.com' + ]); + expect(user3?.stateKey, '@getmeempty:example.com'); + expect(user3?.calcDisplayname(), 'You got me (empty)'); + expect(user3?.membership, Membership.leave); + + // Yield for the onUpdate + await Future.delayed(Duration( + milliseconds: 1, + )); + expect(called.length, 3); + expect(called2.length, 3); + + await subscription.cancel(); + await subscription2.cancel(); }); test('setAvatar', () async { @@ -1319,10 +1390,12 @@ void main() { }); test('inviteLink', () async { // ensure we don't rerequest members - room.summary.mJoinedMemberCount = 4; + room.summary.mJoinedMemberCount = 3; + var matrixToLink = await room.matrixToInviteLink(); expect(matrixToLink.toString(), 'https://matrix.to/#/%23testalias%3Aexample.com'); + room.setState( Event( senderId: '@test:example.com', @@ -1333,9 +1406,10 @@ void main() { originServerTs: DateTime.now(), stateKey: ''), ); + matrixToLink = await room.matrixToInviteLink(); expect(matrixToLink.toString(), - 'https://matrix.to/#/!localpart%3Aserver.abc?via=example.com&via=test.abc&via=example.org'); + 'https://matrix.to/#/!localpart%3Aserver.abc?via=fakeServer.notExisting&via=matrix.org&via=test.abc'); }); test('EventTooLarge on exceeding max PDU size', () async {