Bug: broadcasting roles for MUC are not loaded correctly from DB

(All code is taken from the trunk version of MUCPersistenceManager.java)

When MUC rooms are saved to DB this function is used:

private static int marshallRolesToBroadcast(MUCRoom room) {

StringBuilder buffer = new StringBuilder();

buffer.append((room.canBroadcastPresence(“moderator”) ? “1” : “0”));

buffer.append((room.canBroadcastPresence(“participant”) ? “1” : “0”));

buffer.append((room.canBroadcastPresence(“visitor”) ? “1” : “0”));

return Integer.parseInt(buffer.toString(), 2);

}

Imagine we have a room with “participant” set and nothing else.

So we get a String of “010” which is saved as “2” in DB.

When the room is loaded from DB this part of code is used:

String roles = Integer.toBinaryString(rs.getInt(18));

if (roles.charAt(0) == ‘1’) {

rolesToBroadcast.add(“moderator”);

}

if (roles.length() > 1 && roles.charAt(1) == ‘1’) {

rolesToBroadcast.add(“participant”);

}

if (roles.length() > 2 && roles.charAt(2) == ‘1’) {

rolesToBroadcast.add(“visitor”);

}

So we get a “10” String from the “2” in the DB.

Because of the missing leading “0” the broadcast for “moderator” gets set instead of “participant”.

So you will never get the right roles set for a room when the role “moderator” is not saved to db (because of the missing leading “0”)

The code should look like this instead:

String roles = Integer.toBinaryString(rs.getInt(18));

if (roles.length() > 2 && roles.charAt(roles.length()-3) == ‘1’) {

rolesToBroadcast.add(“moderator”);

}

if (roles.length() > 1 && roles.charAt(roles.length()-2) == ‘1’) {

rolesToBroadcast.add(“participant”);

}

if (roles.charAt(roles.length()-1) == ‘1’) {

rolesToBroadcast.add(“visitor”);

}

Another thing i want to mention:

You should rethink whether it is necessary to build a binary string to save the roles in DB and to create a binary string and doing this compare mess when the room is loaded from DB. This is horribly slow and hard to maintain in comparison to a logical “and” with the roles in the enum.

1 Like

Right, this is a bug, loged as OF-562.

For convenience here are the refered code parts: marshallRolesToBroadcast() and loadRoomsFromDB().

Flow

@Flow: don’t miss the function “loadFromDB()” !

I attached a diff but please keep in mind: You should refactor this part of code to get rid of this crazy binary string thingy.
MUCPersistenceManager.diff.zip (605 Bytes)