fix: startDirectChat might return an unjoined room

If you create DM room, but the other party doesn't join it, they might
be unable to create a new DM using startDirectChat. Since creating a new
DM is pretty much the same as joining the invite, we try to join the
pending DM invite first and fall back to creating a new room if that
fails.

lib/src/client.dart

@@ -611,7 +611,29 @@ class Client extends MatrixApi {
   }) async {
     // Try to find an existing direct chat
     final directChatRoomId = getDirectChatFromUserId(mxid);
-    if (directChatRoomId != null) return directChatRoomId;
+    if (directChatRoomId != null) {
+      final room = getRoomById(directChatRoomId);
+      if (room != null) {
+        if (room.membership == Membership.join) {
+          return directChatRoomId;
+        } else if (room.membership == Membership.invite) {
+          // we might already have an invite into a DM room. If that is the case, we should try to join. If the room is
+          // unjoinable, that will automatically leave the room, so in that case we need to continue creating a new
+          // room. (This implicitly also prevents the room from being returned as a DM room by getDirectChatFromUserId,
+          // because it only returns joined or invited rooms atm.)
+          await room.join();
+          if (room.membership != Membership.leave) {
+            if (waitForSync) {
+              if (room.membership != Membership.join) {
+                // Wait for room actually appears in sync with the right membership
+                await waitForRoomInSync(directChatRoomId, join: true);
+              }
+            }
+            return directChatRoomId;
+          }
+        }
+      }
+    }
 
     enableEncryption ??=
         encryptionEnabled && await userOwnsEncryptionKeys(mxid);
@@ -636,9 +658,12 @@ class Client extends MatrixApi {
       powerLevelContentOverride: powerLevelContentOverride,
     );
 
-    if (waitForSync && getRoomById(roomId) == null) {
-      // Wait for room actually appears in sync
-      await waitForRoomInSync(roomId, join: true);
+    if (waitForSync) {
+      final room = getRoomById(roomId);
+      if (room == null || room.membership != Membership.join) {
+        // Wait for room actually appears in sync
+        await waitForRoomInSync(roomId, join: true);
+      }
     }
 
     await Room(id: roomId, client: this).addToDirectChat(mxid);

test_driver/matrixsdk_test.dart

@@ -420,6 +420,96 @@ void main() => group('Integration tests', () {
         }
         return;
       });
+
+      test('dm creation', () async {
+        Client? testClientA, testClientB;
+
+        try {
+          Hive.init(null);
+
+          await olm.init();
+          olm.Account();
+          Logs().i('[LibOlm] Enabled');
+
+          final homeserverUri = Uri.parse(homeserver);
+          Logs().i('++++ Using homeserver $homeserverUri ++++');
+
+          Logs().i('++++ Login Alice at ++++');
+          testClientA = Client('TestClientA', databaseBuilder: getDatabase);
+          await testClientA.checkHomeserver(homeserverUri);
+          await testClientA.login(
+            LoginType.mLoginPassword,
+            identifier: AuthenticationUserIdentifier(user: Users.user1.name),
+            password: Users.user1.password,
+          );
+          expect(testClientA.encryptionEnabled, true);
+
+          Logs().i('++++ Login Bob ++++');
+          testClientB = Client('TestClientB', databaseBuilder: getDatabase);
+          await testClientB.checkHomeserver(homeserverUri);
+          await testClientB.login(
+            LoginType.mLoginPassword,
+            identifier: AuthenticationUserIdentifier(user: Users.user2.name),
+            password: Users.user2.password,
+          );
+          expect(testClientB.encryptionEnabled, true);
+
+          Logs().i('++++ (Alice) Leave all rooms ++++');
+          while (testClientA.rooms.isNotEmpty) {
+            final room = testClientA.rooms.first;
+            if (room.canonicalAlias.isNotEmpty) {
+              break;
+            }
+            try {
+              await room.leave();
+              await room.forget();
+            } catch (_) {}
+          }
+
+          Logs().i('++++ (Bob) Leave all rooms ++++');
+          for (var i = 0; i < 3; i++) {
+            if (testClientB.rooms.isNotEmpty) {
+              final room = testClientB.rooms.first;
+              try {
+                await room.leave();
+                await room.forget();
+              } catch (_) {}
+            }
+          }
+
+          Logs().i('++++ (Alice) Create DM ++++');
+          final dmRoom = await testClientA.startDirectChat(testClientB.userID!);
+          // conduit returns the room on sync first, so we check if it has already been returned first.
+          if (testClientB.getRoomById(dmRoom)?.membership !=
+              Membership.invite) {
+            await testClientB.waitForRoomInSync(dmRoom, invite: true);
+          }
+
+          Logs().i('++++ (Bob) Create DM ++++');
+          final dmRoomFromB =
+              await testClientB.startDirectChat(testClientA.userID!);
+
+          expect(dmRoom, dmRoomFromB,
+              reason:
+                  "Bob should join alice's DM room instead of creating a new one");
+          expect(testClientB.getRoomById(dmRoom)?.membership, Membership.join,
+              reason: 'Room should actually be in the join state now.');
+          expect(testClientA.getRoomById(dmRoom)?.membership, Membership.join,
+              reason: 'Room should actually be in the join state now.');
+        } catch (e, s) {
+          Logs().e('Test failed', e, s);
+          rethrow;
+        } finally {
+          Logs().i('++++ Logout Alice and Bob ++++');
+          if (testClientA?.isLogged() ?? false) await testClientA!.logoutAll();
+          if (testClientA?.isLogged() ?? false) await testClientB!.logoutAll();
+          await testClientA?.dispose(closeDatabase: false);
+          await testClientB?.dispose(closeDatabase: false);
+          testClientA = null;
+          testClientB = null;
+        }
+        return;
+      });
     }, timeout: Timeout(Duration(minutes: 6)));
 
 Object get olmLengthMatcher {