From 306e12b24e13b3c0ab84e9b3fcd1ca4f8dd8619f Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 10 Nov 2018 17:33:24 +0100 Subject: [PATCH] fixed race condition that prevented bookmark nick to be used --- .../conversations/entities/Bookmark.java | 12 +++++++++- .../conversations/entities/MucOptions.java | 22 +++++++++++++++---- .../services/XmppConnectionService.java | 15 ++++++++----- .../ui/StartConversationActivity.java | 2 +- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/entities/Bookmark.java b/src/main/java/eu/siacs/conversations/entities/Bookmark.java index 748cab716..ebb5de671 100644 --- a/src/main/java/eu/siacs/conversations/entities/Bookmark.java +++ b/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 getTags(Context context) { ArrayList 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() { diff --git a/src/main/java/eu/siacs/conversations/entities/MucOptions.java b/src/main/java/eu/siacs/conversations/entities/MucOptions.java index 952f755c5..7b9613a62 100644 --- a/src/main/java/eu/siacs/conversations/entities/MucOptions.java +++ b/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 { diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index e33870dae..207ea2405 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/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 callback) { diff --git a/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java b/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java index 9c5bf2a7d..03a1fe9ff 100644 --- a/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java +++ b/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;