Merge pull request #1068 from matrix-org/manu/4204_wrong_room_members

MXRoomMembers: Fix wrong view of room members when paginating

CHANGES.rst

@@ -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
  * 

MatrixSDK/Data/MXRoomMembers.h

@@ -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.
  */
@@ -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

MatrixSDK/Data/MXRoomMembers.m

@@ -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.
@@ -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];
@@ -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;
@@ -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;
     }
 
@@ -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
@@ -207,7 +209,7 @@ - (NSString*)memberSortedName:(NSString*)userId
                 }
             }
 
-            [membersWithMembershipDict removeObjectForKey:state.conferenceUserId];
+            [membersWithMembershipDict removeObjectForKey:conferenceUserId];
             membersWithMembership = membersWithMembershipDict.allValues;
         }
     }
@@ -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
@@ -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];
@@ -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
@@ -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

MatrixSDK/Data/MXRoomState.m

@@ -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];
 

MatrixSDKTests/MXRoomStateTests.m

@@ -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];
         }];
     }];