refactored user handling in conferences. show try again button when conference has errors

src/main/java/eu/siacs/conversations/entities/MucOptions.java

@@ -3,7 +3,10 @@ package eu.siacs.conversations.entities;
 import android.annotation.SuppressLint;
 
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 
 import eu.siacs.conversations.R;
 import eu.siacs.conversations.xmpp.forms.Data;
@@ -64,7 +67,7 @@ public class MucOptions {
 		PARTICIPANT("participant", R.string.participant,2),
 		NONE("none", R.string.no_role,0);
 
-		private Role(String string, int resId, int rank) {
+		Role(String string, int resId, int rank) {
 			this.string = string;
 			this.resId = resId;
 			this.rank = rank;
@@ -94,6 +97,7 @@ public class MucOptions {
 	public static final int ERROR_PASSWORD_REQUIRED = 3;
 	public static final int ERROR_BANNED = 4;
 	public static final int ERROR_MEMBERS_ONLY = 5;
+	public static final int ERROR_NO_RESPONSE = 6;
 
 	public static final int KICKED_FROM_ROOM = 9;
 
@@ -236,12 +240,12 @@ public class MucOptions {
 	}
 
 	private Account account;
-	private final List<User> users = new ArrayList<>();
+	private final Map<String, User> users = Collections.synchronizedMap(new LinkedHashMap<String, User>());
 	private List<String> features = new ArrayList<>();
 	private Data form = new Data();
 	private Conversation conversation;
 	private boolean isOnline = false;
-	private int error = ERROR_UNKNOWN;
+	private int error = ERROR_NO_RESPONSE;
 	public OnRenameListener onRenameListener = null;
 	private User self;
 	private String subject = null;
@@ -303,40 +307,15 @@ public class MucOptions {
 	}
 
 	public User deleteUser(String name) {
-		synchronized (this.users) {
-			for (int i = 0; i < users.size(); ++i) {
-				if (users.get(i).getName().equals(name)) {
-					return users.remove(i);
-				}
-			}
-		}
-		return null;
+		return this.users.remove(name);
 	}
 
 	public void addUser(User user) {
-		synchronized (this.users) {
-			for (int i = 0; i < users.size(); ++i) {
-				if (users.get(i).getName().equals(user.getName())) {
-					users.set(i, user);
-					return;
-				}
-			}
-			users.add(user);
-		}
+		this.users.put(user.getName(), user);
 	}
 
 	public User findUser(String name) {
-		if (name == null) {
-			return null;
-		}
-		synchronized (this.users) {
-			for (User user : users) {
-				if (user.getName().equals(name)) {
-					return user;
-				}
-			}
-		}
-		return null;
+		return this.users.get(name);
 	}
 
 	public boolean isUserInRoom(String name) {
@@ -344,26 +323,34 @@ public class MucOptions {
 	}
 
 	public void setError(int error) {
-		this.isOnline = error == ERROR_NO_ERROR;
+		this.isOnline = isOnline && error == ERROR_NO_ERROR;
 		this.error = error;
 	}
 
+	public void setOnline() {
+		this.isOnline = true;
+	}
+
 	public ArrayList<User> getUsers() {
-		synchronized (this.users) {
-			return new ArrayList(this.users);
-		}
+		return new ArrayList<>(users.values());
 	}
 
 	public List<User> getUsers(int max) {
-		synchronized (this.users) {
-			return new ArrayList<>(users.subList(0,Math.min(users.size(),5)));
+		ArrayList<User> users = new ArrayList<>();
+		int i = 1;
+		for(User user : this.users.values()) {
+			users.add(user);
+			if (i >= max) {
+				break;
+			} else {
+				++i;
+			}
 		}
+		return users;
 	}
 
 	public int getUserCount() {
-		synchronized (this.users) {
-			return this.users.size();
-		}
+		return this.users.size();
 	}
 
 	public String getProposedNick() {
@@ -400,7 +387,7 @@ public class MucOptions {
 
 	public void setOffline() {
 		this.users.clear();
-		this.error = 0;
+		this.error = ERROR_NO_RESPONSE;
 		this.isOnline = false;
 	}
 
@@ -417,38 +404,34 @@ public class MucOptions {
 	}
 
 	public String createNameFromParticipants() {
-		synchronized (this.users) {
-			if (users.size() >= 2) {
-				List<String> names = new ArrayList<String>();
-				for (User user : users) {
-					Contact contact = user.getContact();
-					if (contact != null && !contact.getDisplayName().isEmpty()) {
-						names.add(contact.getDisplayName().split("\\s+")[0]);
-					} else {
-						names.add(user.getName());
-					}
+		if (users.size() >= 2) {
+			List<String> names = new ArrayList<>();
+			for (User user : getUsers(5)) {
+				Contact contact = user.getContact();
+				if (contact != null && !contact.getDisplayName().isEmpty()) {
+					names.add(contact.getDisplayName().split("\\s+")[0]);
+				} else {
+					names.add(user.getName());
 				}
-				StringBuilder builder = new StringBuilder();
-				for (int i = 0; i < names.size(); ++i) {
-					builder.append(names.get(i));
-					if (i != names.size() - 1) {
-						builder.append(", ");
-					}
-				}
-				return builder.toString();
-			} else {
-				return null;
 			}
+			StringBuilder builder = new StringBuilder();
+			for (int i = 0; i < names.size(); ++i) {
+				builder.append(names.get(i));
+				if (i != names.size() - 1) {
+					builder.append(", ");
+				}
+			}
+			return builder.toString();
+		} else {
+			return null;
 		}
 	}
 
 	public long[] getPgpKeyIds() {
 		List<Long> ids = new ArrayList<>();
-		synchronized (this.users) {
-			for (User user : this.users) {
-				if (user.getPgpKeyId() != 0) {
-					ids.add(user.getPgpKeyId());
-				}
+		for (User user : this.users.values()) {
+			if (user.getPgpKeyId() != 0) {
+				ids.add(user.getPgpKeyId());
 			}
 		}
 		ids.add(account.getPgpId());
@@ -460,22 +443,18 @@ public class MucOptions {
 	}
 
 	public boolean pgpKeysInUse() {
-		synchronized (this.users) {
-			for (User user : this.users) {
-				if (user.getPgpKeyId() != 0) {
-					return true;
-				}
+		for (User user : this.users.values()) {
+			if (user.getPgpKeyId() != 0) {
+				return true;
 			}
 		}
 		return false;
 	}
 
 	public boolean everybodyHasKeys() {
-		synchronized (this.users) {
-			for (User user : this.users) {
-				if (user.getPgpKeyId() == 0) {
-					return false;
-				}
+		for (User user : this.users.values()) {
+			if (user.getPgpKeyId() == 0) {
+				return false;
 			}
 		}
 		return true;
@@ -489,15 +468,9 @@ public class MucOptions {
 		}
 	}
 
-	public Jid getTrueCounterpart(String counterpart) {
-		synchronized (this.users) {
-			for (User user : this.users) {
-				if (user.getName().equals(counterpart)) {
-					return user.getJid();
-				}
-			}
-		}
-		return null;
+	public Jid getTrueCounterpart(String name) {
+		User user = findUser(name);
+		return user == null ? null : user.getJid();
 	}
 
 	public String getPassword() {

src/main/java/eu/siacs/conversations/parser/PresenceParser.java

@@ -1,8 +1,12 @@
 package eu.siacs.conversations.parser;
 
+import android.util.Log;
+
 import java.util.ArrayList;
 import java.util.List;
 
+
+import eu.siacs.conversations.Config;
 import eu.siacs.conversations.crypto.PgpEngine;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.entities.Contact;
@@ -35,6 +39,7 @@ public class PresenceParser extends AbstractParser implements
 			processConferencePresence(packet, mucOptions);
 			final List<MucOptions.User> tileUserAfter = mucOptions.getUsers(5);
 			if (!tileUserAfter.equals(tileUserBefore)) {
+				Log.d(Config.LOGTAG,account.getJid().toBareJid()+": update tiles for "+conversation.getName());
 				mXmppConnectionService.getAvatarService().clear(mucOptions);
 			}
 			if (before != mucOptions.online() || (mucOptions.online() && count != mucOptions.getUserCount())) {
@@ -56,12 +61,13 @@ public class PresenceParser extends AbstractParser implements
 				if (x != null) {
 					Element item = x.findChild("item");
 					if (item != null && !from.isBareJid()) {
+						mucOptions.setError(MucOptions.ERROR_NO_ERROR);
 						MucOptions.User user = new MucOptions.User(mucOptions,from);
 						user.setAffiliation(item.getAttribute("affiliation"));
 						user.setRole(item.getAttribute("role"));
 						user.setJid(item.getAttributeAsJid("jid"));
 						if (codes.contains(MucOptions.STATUS_CODE_SELF_PRESENCE) || packet.getFrom().equals(mucOptions.getConversation().getJid())) {
-							mucOptions.setError(MucOptions.ERROR_NO_ERROR);
+							mucOptions.setOnline();
 							mucOptions.setSelf(user);
 							if (mucOptions.mNickChangingInProgress) {
 								if (mucOptions.onRenameListener != null) {
@@ -108,6 +114,7 @@ public class PresenceParser extends AbstractParser implements
 						mucOptions.setError(MucOptions.ERROR_MEMBERS_ONLY);
 					} else {
 						mucOptions.setError(MucOptions.ERROR_UNKNOWN);
+						Log.d(Config.LOGTAG, "unknown error in conference: " + packet);
 					}
 				} else if (!from.isBareJid()){
 					MucOptions.User user = mucOptions.deleteUser(from.getResourcepart());

src/main/java/eu/siacs/conversations/ui/ConversationFragment.java

@@ -769,7 +769,7 @@ public class ConversationFragment extends Fragment implements EditMessage.Keyboa
 				case MucOptions.ERROR_NICK_IN_USE:
 					showSnackbar(R.string.nick_in_use, R.string.edit, clickToMuc);
 					break;
-				case MucOptions.ERROR_UNKNOWN:
+				case MucOptions.ERROR_NO_RESPONSE:
 					showSnackbar(R.string.conference_not_found, R.string.leave, leaveMuc);
 					break;
 				case MucOptions.ERROR_PASSWORD_REQUIRED:
@@ -784,6 +784,9 @@ public class ConversationFragment extends Fragment implements EditMessage.Keyboa
 				case MucOptions.KICKED_FROM_ROOM:
 					showSnackbar(R.string.conference_kicked, R.string.join, joinMuc);
 					break;
+				case MucOptions.ERROR_UNKNOWN:
+					showSnackbar(R.string.conference_unknown_error, R.string.try_again, joinMuc);
+					break;
 				default:
 					break;
 			}

src/main/res/values/strings.xml

@@ -242,6 +242,7 @@
 	<string name="you">You</string>
 	<string name="action_edit_subject">Edit conference subject</string>
 	<string name="conference_not_found">Conference not found</string>
+	<string name="conference_unknown_error">Unknown error received</string>2
 	<string name="leave">Leave</string>
 	<string name="contact_added_you">Contact added you to contact list</string>
 	<string name="add_back">Add back</string>