1


Working with Smack 4.3.0 in a Multi User Chat (XEP-0045-1.21) I'm trying to find out if a room is already created or not, but I'm not sure if what I'm doing is the correct way. I have search for it and the most relative answer to it was does MUC exist?.

Technically speaking:

  1. Rooms are created as public and members-only by default in OpenFire 4.2.0.
  2. All room's names are an id defined by the members name in a hash string i.e. XXXXXX029d8c36b62259d0eXXXXXXXX. This means that user A can create a room with B, C and get a groupId like the previous one, but then user B (in another device) can try to create same room (with users A,B,C), which is going to give him same groupId.
  3. Exist a architecture layer like whatsapp, so users can leave a Group Chat and rejoin whenever they want.

What I'm doing at this moment:

@WorkerThread
public boolean isGroupChatAlreadyCreated(@NonNull final String groupId)
        throws
        XmppStringprepException,
        XMPPException.XMPPErrorException,
        MultiUserChatException.NotAMucServiceException,
        SmackException.NotConnectedException,
        InterruptedException,
        SmackException.NoResponseException {
    List<HostedRoom> hostedRooms = manager.getHostedRooms(JidCreate.domainBareFrom(serviceDomain));
    for (HostedRoom hostedRoom : hostedRooms) {
        if (hostedRoom.getName().equalsIgnoreCase(groupId)) {
            return true;
        }
    }

    return false;
}

where manager is MultiUserChatManager manager and serviceDomain is just a String.

so, the questions: is this right way to do it? can it be improved?

MiguelHincapieC
  • 5,445
  • 7
  • 41
  • 72

2 Answers2

2

I believe the easier way is get some info about room, for example its configuration form. If nothing will be returned then it means room does not exist:

public boolean isGroupChatAlreadyCreated(@NonNull final EntityBareJid groupId)
        throws
        XMPPErrorException,
        NotConnectedException,
        InterruptedException,
        NoResponseException {

    MultiUserChat multiUserChat = MultiUserChatManager.getInstanceFor(connection).getMultiUserChat(groupId);

    return multiUserChat.getConfigurationForm() != null;
}

More info here https://github.com/igniterealtime/Smack/blob/4.3/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java#L809

Rubycon
  • 18,156
  • 10
  • 49
  • 70
1

It is essentially the right way.

Ideally you simply use MulitUserChatManager.getRoomInfo(EntityBareJid). The method will return a RoomInfo if the room exists, or throw if it does not.

Your original approach can also be improved by changing the type of 'groupId' to EntityBareJid using equals() instead of equalsIgnoreCase(). And putting your groupId as Localpart of the MUC's address. So your function becomes:

public boolean isGroupChatAlreadyCreated(@NonNull final EntityBareJid groupId)
        throws
        XmppStringprepException,
        XMPPErrorException,
        NotAMucServiceException,
        NotConnectedException,
        InterruptedException,
        NoResponseException {
    List<HostedRoom> hostedRooms = manager.getHostedRooms(JidCreate.domainBareFrom(serviceDomain));
    for (HostedRoom hostedRoom : hostedRooms) {
        if (hostedRoom.getJid().equals(groupId)) {
            return true;
        }
    }

    return false;
}
Flow
  • 23,572
  • 15
  • 99
  • 156
  • I believe it's not an optimal one because what will happen if your server has 100K rooms? You will have a 100K iterations loop each time at client side which can potentially lead to ANR which is not good. I have an alternative suggestion (provided in a separate answer) – Rubycon Sep 14 '18 at 12:05
  • Good point. I hadn't considered large result sets. I would probably use XEP-0045 § 6.4. Will update my answer later. – Flow Sep 14 '18 at 13:23
  • In my case we will not have so many rooms and plus I marked that method with `@WorkerThread`. In the worst case I can implement a binary search, but I'm pretty sure we won't need it (in our case). Anyway I will implement the best one :D – MiguelHincapieC Sep 14 '18 at 14:19
  • Right. If you get an ANR then you are doing it wrong anyways. Still, if XEP-0045 provides a O(1) mechanism to lookup a room, then we should aim for it, instead of the O(n) approach. – Flow Sep 14 '18 at 14:47