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

BF: Left room is still displayed as "Empty room" in rooms list #589

Merged
merged 2 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
BF: Left room is still displayed as "Empty room" in rooms list
  • Loading branch information
manuroe committed Oct 31, 2018
commit 2ee14c6373a9daa04d38dc35fa56c5bb2ae8eb72
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Improvements:
Bug fix:
* MXEvent: Move `invite_room_state` to the correct place in the client-server API (vector-im/riot-ios/issues/2010).
* MXRoomSummaryUpdater: Fix minor issue in updateSummaryAvatar method.
* Left room is still displayed as "Empty room" in rooms list (vector-im/riot-ios/issues/2082).

Changes in Matrix iOS SDK in 0.11.5 (2018-10-05)
===============================================
Expand Down
6 changes: 5 additions & 1 deletion MatrixSDK/Data/MXEventTimeline.m
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,11 @@ - (MXHTTPOperation *)paginate:(NSUInteger)numItems direction:(MXTimelineDirectio

NSLog(@"[MXEventTimeline] paginate : got %tu messages from the server", paginatedResponse.chunk.count);

[self handlePaginationResponse:paginatedResponse direction:direction];
// Check if the room has not been left while waiting for the response
if ([self->room.mxSession hasRoomWithRoomId:self->room.roomId])
{
[self handlePaginationResponse:paginatedResponse direction:direction];
}

// Inform the method caller
complete();
Expand Down
14 changes: 9 additions & 5 deletions MatrixSDK/Data/MXRoom.m
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,16 @@ - (MXHTTPOperation*)members:(void (^)(MXRoomMembers *members))success
}
roomMemberEvents = updatedRoomMemberEvents;

[liveTimeline handleLazyLoadedStateEvents:roomMemberEvents];

[self.mxSession.store storeHasLoadedAllRoomMembersForRoom:self.roomId andValue:YES];
if ([self.mxSession.store respondsToSelector:@selector(commit)])
// Check if the room has not been left while waiting for the response
if ([self.mxSession hasRoomWithRoomId:self.roomId])
{
[self.mxSession.store commit];
[liveTimeline handleLazyLoadedStateEvents:roomMemberEvents];

[self.mxSession.store storeHasLoadedAllRoomMembersForRoom:self.roomId andValue:YES];
if ([self.mxSession.store respondsToSelector:@selector(commit)])
{
[self.mxSession.store commit];
}
}

// Provide the timelime to pending requesters
Expand Down
9 changes: 9 additions & 0 deletions MatrixSDK/MXSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,15 @@ typedef void (^MXOnBackgroundSyncFail)(NSError *error);


#pragma mark - The user's rooms
/**
Check if the user is in a room

@param roomId The room id to the room.

@return YES if they are.
*/
- (BOOL)hasRoomWithRoomId:(NSString*)roomId;

/**
Get the MXRoom instance of a room.

Expand Down
5 changes: 5 additions & 0 deletions MatrixSDK/MXSession.m
Original file line number Diff line number Diff line change
Expand Up @@ -1867,6 +1867,11 @@ - (MXHTTPOperation*)leaveRoom:(NSString*)roomId


#pragma mark - The user's rooms
- (BOOL)hasRoomWithRoomId:(NSString*)roomId
{
return (rooms[roomId] != nil);
}

- (MXRoom *)roomWithRoomId:(NSString *)roomId
{
// sanity check
Expand Down
138 changes: 138 additions & 0 deletions MatrixSDKTests/MXLazyLoadingTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#import "MXSDKOptions.h"

#import "MXHTTPClient_Private.h"
#import "MXFileStore.h"

// Do not bother with retain cycles warnings in tests
#pragma clang diagnostic push
Expand Down Expand Up @@ -790,6 +791,143 @@ - (void)testRoomAfterLeavingFromAnotherDeviceWithLazyLoadingOFF
}


// Complementary test to testRoomAfterLeavingFromAnotherDevice to check
// the regression described at https://github.com/vector-im/riot-ios/issues/2082:
// - Run the scenario
// - Restart Alice session with a permanent store (MXFileStore)
// - Alice requests all members from the HS
// - and make a pagination request, because there is a bug here too
// - Meanwhile, Alice leaves the room
// - Close and reopen Alice session
// -> Alice must not know the room anymore
- (void)checkRoomAfterLeavingWithLazyLoading:(BOOL)lazyLoading
{
// - Run the scenario
[self createScenarioWithLazyLoading:lazyLoading readyToTest:^(MXSession *aliceSession, MXSession *bobSession, MXSession *charlieSession, NSString *roomId, XCTestExpectation *expectation) {

MXRoom *room = [aliceSession roomWithRoomId:roomId];
MXRoomSummary *summary = [aliceSession roomSummaryWithRoomId:roomId];
XCTAssertNotNil(room);
XCTAssertNotNil(summary);

// - Restart Alice session with a permanent store (MXFileStore)
MXRestClient *aliceRestClient = aliceSession.matrixRestClient;
[aliceSession close];
aliceSession = nil;

MXFileStore *store = [[MXFileStore alloc] init];
__block MXSession *aliceSession2 = [[MXSession alloc] initWithMatrixRestClient:aliceRestClient];
[aliceSession2 setStore:store success:^{
[aliceSession2 start:^{

MXRoom *room2 = [aliceSession2 roomWithRoomId:roomId];
MXRoomSummary *summary2 = [aliceSession2 roomSummaryWithRoomId:roomId];
XCTAssertNotNil(room2);
XCTAssertNotNil(summary2);

[room2 liveTimeline:^(MXEventTimeline *liveTimeline) {

// - Alice requests all members from the HS
// Force [MXRoom members:] to make a request
[aliceSession2.store storeHasLoadedAllRoomMembersForRoom:roomId andValue:NO];

[MXHTTPClient setDelay:1000 toRequestsContainingString:@"/members"];
[room2 members:^(MXRoomMembers *members) {

MXRoom *room2a = [aliceSession2 roomWithRoomId:roomId];
MXRoomSummary *summary2a = [aliceSession2 roomSummaryWithRoomId:roomId];
XCTAssertNil(room2a);
XCTAssertNil(summary2a);

} failure:^(NSError *error) {
XCTFail(@"Cannot set up intial test conditions - error: %@", error);
[expectation fulfill];
}];


// - and a pagination request, because there is a bug here too
[MXHTTPClient setDelay:1000 toRequestsContainingString:@"/messages"];
[liveTimeline resetPagination];
[liveTimeline paginate:30 direction:MXTimelineDirectionBackwards onlyFromStore:NO complete:^{

MXRoom *room2a = [aliceSession2 roomWithRoomId:roomId];
MXRoomSummary *summary2a = [aliceSession2 roomSummaryWithRoomId:roomId];
XCTAssertNil(room2a);
XCTAssertNil(summary2a);

} failure:^(NSError *error) {
XCTFail(@"Cannot set up intial test conditions - error: %@", error);
[expectation fulfill];
}];


// - Meanwhile, she leaves the room
[room2 leave:^{

// Let time for the pagination to complete
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 2000 * USEC_PER_SEC), dispatch_get_main_queue(), ^{

// Keep a ref to room2 so that requests on it can complete
NSLog(@"%@", room2);

MXRoom *room2b = [aliceSession2 roomWithRoomId:roomId];
MXRoomSummary *summary2b = [aliceSession2 roomSummaryWithRoomId:roomId];
XCTAssertNil(room2b);
XCTAssertNil(summary2b);

// - Close and reopen Alice session
[aliceSession2 close];
aliceSession2 = nil;

MXFileStore *store = [[MXFileStore alloc] init];
MXSession *aliceSession3 = [[MXSession alloc] initWithMatrixRestClient:aliceRestClient];
[aliceSession3 setStore:store success:^{
[aliceSession3 start:^{

// -> Alice must not know the room anymore
MXRoom *room3 = [aliceSession3 roomWithRoomId:roomId];
MXRoomSummary *summary3 = [aliceSession3 roomSummaryWithRoomId:roomId];
XCTAssertNil(room3);
XCTAssertNil(summary3);

[expectation fulfill];

} failure:^(NSError *error) {
XCTFail(@"Cannot set up intial test conditions - error: %@", error);
[expectation fulfill];
}];
} failure:^(NSError *error) {
XCTFail(@"Cannot set up intial test conditions - error: %@", error);
[expectation fulfill];
}];
});
} failure:^(NSError *error) {
XCTFail(@"Cannot set up intial test conditions - error: %@", error);
[expectation fulfill];
}];
}];
} failure:^(NSError *error) {
XCTFail(@"Cannot set up intial test conditions - error: %@", error);
[expectation fulfill];
}];
} failure:^(NSError *error) {
XCTFail(@"Cannot set up intial test conditions - error: %@", error);
[expectation fulfill];
}];
}];
}

- (void)testRoomAfterLeaving
{
[self checkRoomAfterLeavingWithLazyLoading:YES];
}

- (void)testRoomAfterLeavingWithLazyLoadingOFF
{
[self checkRoomAfterLeavingWithLazyLoading:NO];
}


// roomSummary.membersCount must be right in both cases
- (void)checkRoomSummaryMembersCountWithLazyLoading:(BOOL)lazyLoading
{
Expand Down