rename in MUC if bookmark nick or default (PEP nick) changes

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

@@ -242,7 +242,7 @@ public class Bookmark extends Element implements ListItem {
 	}
 
 	public String getNick() {
-		return this.findChildContent("nick");
+		return Strings.emptyToNull(this.findChildContent("nick"));
 	}
 
 	public void setNick(String nick) {
@@ -320,7 +320,6 @@ public class Bookmark extends Element implements ListItem {
 			this.conversation = null;
 		} else {
 			this.conversation = new WeakReference<>(conversation);
-			conversation.getMucOptions().notifyOfBookmarkNick(getNick());
 		}
 	}
 

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

@@ -7,6 +7,8 @@ import android.text.TextUtils;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
+import com.google.common.base.Strings;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
@@ -61,12 +63,10 @@ public class MucOptions {
     private User self;
     private String password = null;
 
-    private boolean tookProposedNickFromBookmark = false;
+    public MucOptions(final Conversation conversation) {
-
-    public MucOptions(Conversation conversation) {
         this.account = conversation.getAccount();
         this.conversation = conversation;
-        final String nick = getProposedNick(conversation.getAttribute("mucNick"));
+        final String nick = getProposedNickPure(conversation.getAttribute("mucNick"));
         this.self = new User(this, createJoinJid(nick), null, nick, new HashSet<>());
         this.self.affiliation = Affiliation.of(conversation.getAttribute("affiliation"));
         this.self.role = Role.of(conversation.getAttribute("role"));
@@ -117,17 +117,6 @@ public class MucOptions {
         }
     }
 
-    public boolean isTookProposedNickFromBookmark() {
-        return tookProposedNickFromBookmark;
-    }
-
-    void notifyOfBookmarkNick(final String nick) {
-        final String normalized = normalize(account.getJid(),nick);
-        if (normalized != null && normalized.equals(getSelf().getNick())) {
-            this.tookProposedNickFromBookmark = true;
-        }
-    }
-
     public boolean mamSupport() {
         return MessageArchiveService.Version.has(getFeatures());
     }
@@ -139,8 +128,8 @@ public class MucOptions {
         if (roomConfigName != null) {
             name = roomConfigName.getValue();
         } else {
-            List<ServiceDiscoveryResult.Identity> identities = serviceDiscoveryResult.getIdentities();
+            final var identities = serviceDiscoveryResult.getIdentities();
-            String identityName = identities.size() > 0 ? identities.get(0).getName() : null;
+            final String identityName = !identities.isEmpty() ? identities.get(0).getName() : null;
             final Jid jid = conversation.getJid();
             if (identityName != null && !identityName.equals(jid == null ? null : jid.getEscapedLocal())) {
                 name = identityName;
@@ -157,7 +146,7 @@ public class MucOptions {
 
     private Data getRoomInfoForm() {
         final List<Data> forms = serviceDiscoveryResult == null ? Collections.emptyList() : serviceDiscoveryResult.forms;
-        return forms.size() == 0 ? new Data() : forms.get(0);
+        return forms.isEmpty() ? new Data() : forms.get(0);
     }
 
     public String getAvatar() {
@@ -500,20 +489,31 @@ public class MucOptions {
         }
     }
 
-    public String getProposedNick() {
+    private String getProposedNick() {
-        return getProposedNick(null);
+        final Bookmark bookmark = this.conversation.getBookmark();
+        if (bookmark != null) {
+            // if we already have a bookmark we consider this the source of truth
+            return getProposedNickPure();
+        }
+        final var storedJid = conversation.getJid();
+        if (storedJid.isBareJid()) {
+            return defaultNick(account);
+        } else {
+            return storedJid.getResource();
+        }
     }
 
-    public String getProposedNick(final String mucNick) {
+    public String getProposedNickPure() {
+        return getProposedNickPure(null);
+    }
+
+    public String getProposedNickPure(final String mucNick) {
         final Bookmark bookmark = this.conversation.getBookmark();
         final String bookmarkedNick = normalize(account.getJid(), bookmark == null ? null : bookmark.getNick());
         if (bookmarkedNick != null) {
-            this.tookProposedNickFromBookmark = true;
             return bookmarkedNick;
         } else if (mucNick != null) {
             return mucNick;
-        } else if (!conversation.getJid().isBareJid()) {
-            return conversation.getJid().getResource();
         } else {
             return defaultNick(account);
         }
@@ -528,14 +528,14 @@ public class MucOptions {
         }
     }
 
-    private static String normalize(Jid account, String nick) {
+    private static String normalize(final Jid account, final String nick) {
-        if (account == null || TextUtils.isEmpty(nick)) {
+        if (account == null || Strings.isNullOrEmpty(nick)) {
             return null;
         }
 
         try {
             return account.withResource(nick).getResource();
-        } catch (IllegalArgumentException e) {
+        } catch (final IllegalArgumentException e) {
             return nick;
         }
     }

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

@@ -444,12 +444,13 @@ public class MessageParser extends AbstractParser implements Consumer<im.convers
         mXmppConnectionService.processDeletedBookmarks(account, previous);
     }
 
-    private void setNick(Account account, Jid user, String nick) {
+    private void setNick(final Account account, final Jid user, final String nick) {
         if (user.asBareJid().equals(account.getJid().asBareJid())) {
             account.setDisplayName(nick);
             if (QuickConversationsService.isQuicksy()) {
                 mXmppConnectionService.getAvatarService().clear(account);
             }
+            mXmppConnectionService.checkMucRequiresRename();
         } else {
             Contact contact = account.getRoster().getContact(user);
             if (contact.setPresenceName(nick)) {

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

@@ -92,11 +92,13 @@ public class PresenceParser extends AbstractParser implements Consumer<im.conver
                             if (mucOptions.setOnline()) {
                                 mXmppConnectionService.getAvatarService().clear(mucOptions);
                             }
+                            final var current = mucOptions.getSelf().getFullJid();
                             if (mucOptions.setSelf(user)) {
                                 Log.d(Config.LOGTAG,"role or affiliation changed");
                                 mXmppConnectionService.databaseBackend.updateConversation(conversation);
                             }
-                            mXmppConnectionService.persistSelfNick(user);
+                            final var modified = current == null || !current.equals(user.getFullJid());
+                            mXmppConnectionService.persistSelfNick(user, modified);
                             invokeRenameListener(mucOptions, true);
                         }
                         boolean isNew = mucOptions.updateUser(user);

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

@@ -2509,11 +2509,13 @@ public class XmppConnectionService extends Service {
                 final MucOptions mucOptions = conversation.getMucOptions();
                 if (mucOptions.getError() == MucOptions.Error.NICK_IN_USE) {
                     final String current = mucOptions.getActualNick();
-                    final String proposed = mucOptions.getProposedNick();
+                    final String proposed = mucOptions.getProposedNickPure();
                     if (current != null && !current.equals(proposed)) {
                         Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": proposed nick changed after bookmark push " + current + "->" + proposed);
                         joinMuc(conversation);
                     }
+                } else {
+                    checkMucRequiresRename(conversation);
                 }
             }
         } else if (bookmark.autojoin()) {
@@ -3998,29 +4000,34 @@ public class XmppConnectionService extends Service {
         new Thread(() -> onMediaLoaded.onMediaLoaded(fileBackend.convertToAttachments(databaseBackend.getRelativeFilePaths(account, jid, limit)))).start();
     }
 
-    public void persistSelfNick(final MucOptions.User self) {
+    public void persistSelfNick(final MucOptions.User self, final boolean modified) {
         final Conversation conversation = self.getConversation();
-        final boolean tookProposedNickFromBookmark = conversation.getMucOptions().isTookProposedNickFromBookmark();
+        final Account account = conversation.getAccount();
-        Jid full = self.getFullJid();
+        final Jid full = self.getFullJid();
         if (!full.equals(conversation.getJid())) {
-            Log.d(Config.LOGTAG, "nick changed. updating");
+            Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": persisting full jid " + full);
             conversation.setContactJid(full);
             databaseBackend.updateConversation(conversation);
         }
 
-        final String nick = self.getNick();
         final Bookmark bookmark = conversation.getBookmark();
-        final String bookmarkedNick = bookmark == null ? null : bookmark.getNick();
+        if (bookmark == null || !modified) {
-        if (bookmark != null && (tookProposedNickFromBookmark || Strings.isNullOrEmpty(bookmarkedNick)) && !nick.equals(bookmarkedNick)) {
+            return;
-            final Account account = conversation.getAccount();
-            final String defaultNick = MucOptions.defaultNick(account);
-            if (Strings.isNullOrEmpty(bookmarkedNick) && full.getResource().equals(defaultNick)) {
-                return;
-            }
-            Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": persist nick '" + nick + "' into bookmark for " + conversation.getJid().asBareJid());
-            bookmark.setNick(nick);
-            createBookmark(bookmark.getAccount(), bookmark);
         }
+        final var nick = full.getResource();
+        final String defaultNick = MucOptions.defaultNick(account);
+        if (nick.equals(defaultNick) || nick.equals(bookmark.getNick())) {
+            return;
+        }
+        Log.d(
+                Config.LOGTAG,
+                account.getJid().asBareJid()
+                        + ": persist nick '"
+                        + full.getResource()
+                        + "' into bookmark for "
+                        + conversation.getJid().asBareJid());
+        bookmark.setNick(nick);
+        createBookmark(bookmark.getAccount(), bookmark);
     }
 
     public void presenceToMuc(final Conversation conversation) {
@@ -4034,7 +4041,12 @@ public class XmppConnectionService extends Service {
         }
     }
 
-    public boolean renameInMuc(final Conversation conversation, final String nick, final UiCallback<Conversation> callback) {
+    public boolean renameInMuc(
+            final Conversation conversation,
+            final String nick,
+            final UiCallback<Conversation> callback) {
+        final Account account = conversation.getAccount();
+        final Bookmark bookmark = conversation.getBookmark();
         final MucOptions options = conversation.getMucOptions();
         final Jid joinJid = options.createJoinJid(nick);
         if (joinJid == null) {
@@ -4043,7 +4055,6 @@ public class XmppConnectionService extends Service {
         if (options.online()) {
             maybeRegisterWithMuc(conversation, nick);
 
-            Account account = conversation.getAccount();
             options.setOnRenameListener(new OnRenameListener() {
 
                 @Override
@@ -4063,14 +4074,24 @@ public class XmppConnectionService extends Service {
             final var packet = mPresenceGenerator.selfPresence(account, Presence.Status.ONLINE, options.nonanonymous(), nick);
             packet.setTo(joinJid);
             sendPresencePacket(account, packet);
+            if (nick.equals(MucOptions.defaultNick(account))
+                    && bookmark != null
+                    && bookmark.getNick() != null) {
+                Log.d(
+                        Config.LOGTAG,
+                        account.getJid().asBareJid()
+                                + ": removing nick from bookmark for "
+                                + bookmark.getJid());
+                bookmark.setNick(null);
+                createBookmark(account, bookmark);
+            }
         } else {
             conversation.setContactJid(joinJid);
             databaseBackend.updateConversation(conversation);
-            if (conversation.getAccount().getStatus() == Account.State.ONLINE) {
+            if (account.getStatus() == Account.State.ONLINE) {
-                Bookmark bookmark = conversation.getBookmark();
                 if (bookmark != null) {
                     bookmark.setNick(nick);
-                    createBookmark(bookmark.getAccount(), bookmark);
+                    createBookmark(account, bookmark);
                 }
                 joinMuc(conversation);
             }
@@ -4078,6 +4099,40 @@ public class XmppConnectionService extends Service {
         return true;
     }
 
+    public void checkMucRequiresRename() {
+        synchronized (this.conversations) {
+            for (final Conversation conversation : this.conversations) {
+                if (conversation.getMode() == Conversational.MODE_MULTI) {
+                    checkMucRequiresRename(conversation);
+                }
+            }
+        }
+    }
+
+    private void checkMucRequiresRename(final Conversation conversation) {
+        final var options = conversation.getMucOptions();
+        if (!options.online()) {
+            return;
+        }
+        final var account = conversation.getAccount();
+        final String current = options.getActualNick();
+        final String proposed = options.getProposedNickPure();
+        if (current == null || current.equals(proposed)) {
+            return;
+        }
+        final Jid joinJid = options.createJoinJid(proposed);
+        Log.d(
+                Config.LOGTAG,
+                String.format(
+                        "%s: muc rename required %s (was: %s)",
+                        account.getJid().asBareJid(), joinJid, current));
+        final var packet =
+                mPresenceGenerator.selfPresence(
+                        account, Presence.Status.ONLINE, options.nonanonymous(), options.getSelf().getNick());
+        packet.setTo(joinJid);
+        sendPresencePacket(account, packet);
+    }
+
     public void leaveMuc(Conversation conversation) {
         leaveMuc(conversation, false);
     }