fixed race condition that prevented bookmark nick to be used

Daniel Gultsch created 7 years ago

Change summary

src/main/java/eu/siacs/conversations/entities/Bookmark.java              | 12 
src/main/java/eu/siacs/conversations/entities/MucOptions.java            | 22 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java | 15 
src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java   |  2 
4 files changed, 39 insertions(+), 12 deletions(-)

Detailed changes

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

@@ -82,6 +82,11 @@ public class Bookmark extends Element implements ListItem {
 		return this.jid;
 	}
 
+	public Jid getFullJid() {
+		final String nick = getNick();
+		return jid == null || nick == null || nick.trim().isEmpty() ? jid : jid.withResource(nick);
+	}
+
 	@Override
 	public List<Tag> getTags(Context context) {
 		ArrayList<Tag> tags = new ArrayList<>();
@@ -155,7 +160,12 @@ public class Bookmark extends Element implements ListItem {
 		if (this.conversation != null) {
 			this.conversation.clear();
 		}
-		this.conversation = new WeakReference<>(conversation);
+		if (conversation == null) {
+			this.conversation = null;
+		} else {
+			this.conversation = new WeakReference<>(conversation);
+			conversation.getMucOptions().notifyOfBookmarkNick(getNick());
+		}
 	}
 
 	public String getBookmarkName() {

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

@@ -42,6 +42,9 @@ public class MucOptions {
     private Error error = Error.NONE;
     private User self;
     private String password = null;
+
+    private boolean tookProposedNickFromBookmark = false;
+
     public MucOptions(Conversation conversation) {
         this.account = conversation.getAccount();
         this.conversation = conversation;
@@ -94,6 +97,16 @@ public class MucOptions {
         }
     }
 
+    public boolean isTookProposedNickFromBookmark() {
+        return tookProposedNickFromBookmark;
+    }
+
+    void notifyOfBookmarkNick(String nick) {
+        if (nick != null && nick.trim().equals(getSelf().getFullJid().getResource())) {
+            this.tookProposedNickFromBookmark = true;
+        }
+    }
+
     public boolean mamSupport() {
         return MessageArchiveService.Version.has(getFeatures());
     }
@@ -374,10 +387,11 @@ public class MucOptions {
     }
 
     private String getProposedNick() {
-        if (conversation.getBookmark() != null
-                && conversation.getBookmark().getNick() != null
-                && !conversation.getBookmark().getNick().trim().isEmpty()) {
-            return conversation.getBookmark().getNick().trim();
+        final Bookmark bookmark = this.conversation.getBookmark();
+        final String bookmarkedNick = bookmark == null ? null : bookmark.getNick();
+        if (bookmarkedNick != null && !bookmarkedNick.trim().isEmpty()) {
+            this.tookProposedNickFromBookmark = true;
+            return bookmarkedNick.trim();
         } else if (!conversation.getJid().isBareJid()) {
             return conversation.getJid().getResource();
         } else {

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java

@@ -1405,7 +1405,7 @@ public class XmppConnectionService extends Service {
                     if (conversation != null) {
                         bookmark.setConversation(conversation);
                     } else if (bookmark.autojoin() && bookmark.getJid() != null && autojoin) {
-                        conversation = findOrCreateConversation(account, bookmark.getJid(), true, true, false);
+                        conversation = findOrCreateConversation(account, bookmark.getFullJid(), true, true, false);
                         bookmark.setConversation(conversation);
                     }
                 }
@@ -2439,6 +2439,7 @@ public class XmppConnectionService extends Service {
 
 	public void persistSelfNick(MucOptions.User self) {
 		final Conversation conversation = self.getConversation();
+		final boolean tookProposedNickFromBookmark = conversation.getMucOptions().isTookProposedNickFromBookmark();
 		Jid full = self.getFullJid();
 		if (!full.equals(conversation.getJid())) {
 			Log.d(Config.LOGTAG, "nick changed. updating");
@@ -2446,11 +2447,13 @@ public class XmppConnectionService extends Service {
 			databaseBackend.updateConversation(conversation);
 		}
 
-		Bookmark bookmark = conversation.getBookmark();
-		if (bookmark != null && !full.getResource().equals(bookmark.getNick())) {
-			bookmark.setNick(full.getResource());
-			pushBookmarks(bookmark.getAccount());
-		}
+		final Bookmark bookmark = conversation.getBookmark();
+		final String bookmarkedNick = bookmark == null ? null : bookmark.getNick();
+        if (bookmark != null && (tookProposedNickFromBookmark || TextUtils.isEmpty(bookmarkedNick)) && !full.getResource().equals(bookmarkedNick)) {
+            Log.d(Config.LOGTAG, conversation.getAccount().getJid().asBareJid() + ": persist nick '" + full.getResource() + "' into bookmark for " + conversation.getJid().asBareJid());
+            bookmark.setNick(full.getResource());
+            pushBookmarks(bookmark.getAccount());
+        }
 	}
 
 	public boolean renameInMuc(final Conversation conversation, final String nick, final UiCallback<Conversation> callback) {

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

@@ -404,7 +404,7 @@ public class StartConversationActivity extends XmppActivity implements XmppConne
 	}
 
 	protected void openConversationsForBookmark(Bookmark bookmark) {
-		Jid jid = bookmark.getJid();
+		final Jid jid = bookmark.getFullJid();
 		if (jid == null) {
 			Toast.makeText(this, R.string.invalid_jid, Toast.LENGTH_SHORT).show();
 			return;