Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MXRoomMembers: Fix wrong view of room members when paginating #1068

Merged
merged 1 commit into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Changes to be released in next version

🐛 Bugfix
* MXSession: Fix deadlock regression in resume() (vector-im/element-ios/issues/4202).
* MXRoomMembers: Fix wrong view of room members when paginating (vector-im/element-ios/issues/4204).

⚠️ API Changes
*
Expand Down
18 changes: 0 additions & 18 deletions MatrixSDK/Data/MXRoomMembers.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@
@return The newly-initialized MXRoomMembers.
*/
- (instancetype)initWithRoomState:(MXRoomState*)state andMatrixSession:(MXSession*)matrixSession;

/**
The room state belonging to this object.
*/
@property (nonatomic, readonly) MXRoomState *roomState;

/**
A copy of the list of room members.
*/
Expand Down Expand Up @@ -115,16 +109,4 @@
*/
- (BOOL)handleStateEvents:(NSArray<MXEvent *> *)stateEvents;

#pragma mark - NSCopying

/**
Copy the receiver and update the room state reference. This is useful when the copying happens
as part of copying an MXRoomState object.

@param zone a memory zone.
@param state the new/copied room state.
@return a copy of the receiver.
*/
- (id)copyWithZone:(NSZone *)zone andState:(MXRoomState *)state;

@end
65 changes: 43 additions & 22 deletions MatrixSDK/Data/MXRoomMembers.m
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@
@interface MXRoomMembers ()
{
MXSession *mxSession;
__weak MXRoomState *state;

// Context for this MXRoomMembers instance
BOOL isConferenceUserRoom;
NSString *conferenceUserId;
BOOL isLive;

/**
Members ordered by userId.
Expand All @@ -47,19 +51,17 @@ - (instancetype)initWithRoomState:(MXRoomState *)roomState andMatrixSession:(MXS
if (self)
{
mxSession = matrixSession;
state = roomState;

isConferenceUserRoom = roomState.isConferenceUserRoom;
conferenceUserId = roomState.conferenceUserId;
isLive = roomState.isLive;

members = [NSMutableDictionary dictionary];
membersNamesInUse = [NSMutableDictionary dictionary];
}
return self;
}

- (MXRoomState *)roomState
{
return state;
}

- (NSArray<MXRoomMember *> *)members
{
return [members allValues];
Expand Down Expand Up @@ -158,12 +160,12 @@ - (NSString*)memberSortedName:(NSString*)userId
{
NSArray<MXRoomMember *> *membersWithoutConferenceUser;

if (state.isConferenceUserRoom)
if (isConferenceUserRoom)
{
// Show everyone in a 1:1 room with a conference user
membersWithoutConferenceUser = self.members;
}
else if (![self memberWithUserId:state.conferenceUserId])
else if (![self memberWithUserId:conferenceUserId])
{
// There is no conference user. No need to filter
membersWithoutConferenceUser = self.members;
Expand All @@ -172,7 +174,7 @@ - (NSString*)memberSortedName:(NSString*)userId
{
// Filter the conference user from the list
NSMutableDictionary<NSString*, MXRoomMember*> *membersWithoutConferenceUserDict = [NSMutableDictionary dictionaryWithDictionary:members];
[membersWithoutConferenceUserDict removeObjectForKey:state.conferenceUserId];
[membersWithoutConferenceUserDict removeObjectForKey:conferenceUserId];
membersWithoutConferenceUser = membersWithoutConferenceUserDict.allValues;
}

Expand All @@ -183,14 +185,14 @@ - (NSString*)memberSortedName:(NSString*)userId
{
NSArray<MXRoomMember *> *membersWithMembership;

if (includeConferenceUser || state.isConferenceUserRoom)
if (includeConferenceUser || isConferenceUserRoom)
{
// Show everyone in a 1:1 room with a conference user
membersWithMembership = [self membersWithMembership:theMembership];
}
else
{
MXRoomMember *conferenceUserMember = [self memberWithUserId:state.conferenceUserId];
MXRoomMember *conferenceUserMember = [self memberWithUserId:conferenceUserId];
if (!conferenceUserMember || conferenceUserMember.membership != theMembership)
{
// The conference user is not in list of members with the passed membership
Expand All @@ -207,7 +209,7 @@ - (NSString*)memberSortedName:(NSString*)userId
}
}

[membersWithMembershipDict removeObjectForKey:state.conferenceUserId];
[membersWithMembershipDict removeObjectForKey:conferenceUserId];
membersWithMembership = membersWithMembershipDict.allValues;
}
}
Expand Down Expand Up @@ -255,7 +257,7 @@ - (BOOL)handleStateEvents:(NSArray<MXEvent *> *)stateEvents;
}
}

MXRoomMember *roomMember = [[MXRoomMember alloc] initWithMXEvent:event andEventContent:[state contentOfEvent:event]];
MXRoomMember *roomMember = [[MXRoomMember alloc] initWithMXEvent:event andEventContent:[self contentOfEvent:event]];
if (roomMember)
{
/// Update membersNamesInUse
Expand Down Expand Up @@ -292,7 +294,7 @@ - (BOOL)handleStateEvents:(NSArray<MXEvent *> *)stateEvents;

// Special handling for presence: update MXUser data in case of membership event.
// CAUTION: ignore here redacted state event, the redaction concerns only the context of the event room.
if (state.isLive && !event.isRedactedEvent && roomMember.membership == MXMembershipJoin)
if (isLive && !event.isRedactedEvent && roomMember.membership == MXMembershipJoin)
{
MXUser *user = [mxSession getOrCreateUser:event.sender];
[user updateWithRoomMemberEvent:event roomMember:roomMember inMatrixSession:mxSession];
Expand All @@ -314,18 +316,14 @@ - (BOOL)handleStateEvents:(NSArray<MXEvent *> *)stateEvents;

#pragma mark - NSCopying

- (id)copyWithZone:(NSZone *)zone andState:(MXRoomState *)state {
MXRoomMembers *copy = [self copyWithZone:zone];
copy->state = state;
return copy;
}

- (id)copyWithZone:(NSZone *)zone
{
MXRoomMembers *membersCopy = [[MXRoomMembers allocWithZone:zone] init];

membersCopy->mxSession = mxSession;
membersCopy->state = state;
membersCopy->isConferenceUserRoom = isConferenceUserRoom;
membersCopy->conferenceUserId = conferenceUserId;
membersCopy->isLive = isLive;

// MXRoomMember objects in members are immutable. A new instance of it is created each time
// the sdk receives room member event, even if it is an update of an existing member like a
Expand All @@ -336,4 +334,27 @@ - (id)copyWithZone:(NSZone *)zone

return membersCopy;
}


#pragma mark - Private methods

// According to the direction of the instance, we are interested either by
// the content of the event or its prev_content
- (NSDictionary<NSString *, id> *)contentOfEvent:(MXEvent*)event
{
NSDictionary<NSString *, id> *content;
if (event)
{
if (isLive)
{
content = event.content;
}
else
{
content = event.prevContent;
}
}
return content;
}

@end
2 changes: 1 addition & 1 deletion MatrixSDK/Data/MXRoomState.m
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ - (id)copyWithZone:(NSZone *)zone
stateCopy->stateEvents[key] = [[NSMutableArray allocWithZone:zone] initWithArray:stateEvents[key]];
}

stateCopy->_members = [_members copyWithZone:zone andState:stateCopy];
stateCopy->_members = [_members copyWithZone:zone];

stateCopy->_membersCount = [_membersCount copyWithZone:zone];

Expand Down
2 changes: 1 addition & 1 deletion MatrixSDKTests/MXRoomStateTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ - (void)testCopying
mxSession = mxSession2;
[room state:^(MXRoomState *roomState) {
MXRoomState *roomStateCopy = [roomState copy];
XCTAssertEqual(roomStateCopy.members.roomState, roomStateCopy);
XCTAssertEqual(roomStateCopy.members.members.count, roomState.members.members.count);
[expectation fulfill];
}];
}];
Expand Down