[jitsi-dev] [jitsi] Only create the chat room if provider cannot find it. (#28)

#1

When MUCServiceImpl creates a chat room (i.e. a ChatRoomWrapper instance) it should first check to see if the chat room already exists and is already known by the protocol.

By first looking for an existing chat room, findRoom(...), before creating the chat room, we can make sure that we are not redundantly creating a chat room (and causing an OperationFailedException as a consequence).
Also, this becomes particularly relevant when the IRC protocol implementation is merged, since the IRC servers may automatically join a user to a particular channel, in which case the ChatRoom instance (related to the protocol implementation) exists, but the ChatRoomWrapper (related to the Jitsi core framework) does not yet.
You can merge this Pull Request by running:

  git pull https://github.com/cobratbq/jitsi chatroom-fix

Or you can view, comment on it, or merge it online at:

  https://github.com/jitsi/jitsi/pull/28

-- Commit Summary --

  * Only create the chat room if provider cannot find it.

-- File Changes --

    M src/net/java/sip/communicator/impl/muc/MUCServiceImpl.java (17)

-- Patch Links --

https://github.com/jitsi/jitsi/pull/28.patch
https://github.com/jitsi/jitsi/pull/28.diff

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/28

#2

I don't mind merging this, but looking at it again, I wonder if this could yield an exception anyway if a second server command in a multi threaded environment arrives to create a MUC (findRoom already done, but createRoom not yet executed). Wouldn't it make more sense to perform this check inside of groupChatOpSet.createChatRoom(...)?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/28#issuecomment-42130229

#3

Closed #28.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/28

#4

Oops, didn't mean to close the pull request, just the reply.

You are right about the race condition. That should be resolved in some way first.

I am going to have a second look at this. There was also a caveat with going to other way, since that would create instances for chat rooms that don't necessarily exist or have been joined yet. I got some issue with multiple instances for the same chat room and contacts deviating between the two instances.
I am not sure when exactly those occurred anymore. It could've been related to the IRC auto-join behavior initiated by the server. Will get back to you when I got more info.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/28#issuecomment-42137010