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

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Bookmark.java              |   3 
src/main/java/eu/siacs/conversations/entities/MucOptions.java            |  46 
src/main/java/eu/siacs/conversations/parser/MessageParser.java           |   3 
src/main/java/eu/siacs/conversations/parser/PresenceParser.java          |   5 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java | 125 
5 files changed, 120 insertions(+), 62 deletions(-)

Detailed changes

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

@@ -186,7 +186,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) {
@@ -250,7 +250,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 🔗

@@ -53,9 +53,7 @@ public class MucOptions {
     private User self;
     private String password = null;
 
-    private boolean tookProposedNickFromBookmark = false;
-
-    public MucOptions(Conversation conversation) {
+    public MucOptions(final Conversation conversation) {
         this.account = conversation.getAccount();
         this.conversation = conversation;
         this.self = new User(this, createJoinJid(getProposedNick()));
@@ -111,17 +109,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().getFullJid().getResource())) {
-            this.tookProposedNickFromBookmark = true;
-        }
-    }
-
     public boolean mamSupport() {
         return MessageArchiveService.Version.has(getFeatures());
     }
@@ -133,8 +120,8 @@ public class MucOptions {
         if (roomConfigName != null) {
             name = roomConfigName.getValue();
         } else {
-            List<ServiceDiscoveryResult.Identity> identities = serviceDiscoveryResult.getIdentities();
-            String identityName = identities.size() > 0 ? identities.get(0).getName() : null;
+            final var identities = serviceDiscoveryResult.getIdentities();
+            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;
@@ -151,7 +138,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() {
@@ -474,14 +461,25 @@ public class MucOptions {
         }
     }
 
-    public String getProposedNick() {
+    private String getProposedNick() {
+        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 getProposedNickPure() {
         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 (!conversation.getJid().isBareJid()) {
-            return conversation.getJid().getResource();
         } else {
             return defaultNick(account);
         }
@@ -496,13 +494,13 @@ public class MucOptions {
         }
     }
 
-    private static String normalize(Jid account, String nick) {
-        if (account == null || TextUtils.isEmpty(nick)) {
+    private static String normalize(final Jid account, final String nick) {
+        if (account == null || Strings.isNullOrEmpty(nick)) {
             return null;
         }
         try {
             return account.withResource(nick).getResource();
-        } catch (IllegalArgumentException e) {
+        } catch (final IllegalArgumentException e) {
             return null;
         }
 

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

@@ -337,12 +337,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 🔗

@@ -85,13 +85,14 @@ 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 🔗

@@ -1969,11 +1969,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()) {
@@ -3319,62 +3321,85 @@ 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();
-        Jid full = self.getFullJid();
+        final Account account = conversation.getAccount();
+        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 Bookmark bookmark = conversation.getBookmark();
-        final String bookmarkedNick = bookmark == null ? null : bookmark.getNick();
-        if (bookmark != null && (tookProposedNickFromBookmark || Strings.isNullOrEmpty(bookmarkedNick)) && !full.getResource().equals(bookmarkedNick)) {
-            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 '" + full.getResource() + "' into bookmark for " + conversation.getJid().asBareJid());
-            bookmark.setNick(full.getResource());
-            createBookmark(bookmark.getAccount(), bookmark);
+        if (bookmark == null || !modified) {
+            return;
         }
-    }
-
-    public boolean renameInMuc(final Conversation conversation, final String nick, final UiCallback<Conversation> callback) {
+        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 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) {
             return false;
         }
         if (options.online()) {
-            Account account = conversation.getAccount();
-            options.setOnRenameListener(new OnRenameListener() {
+            options.setOnRenameListener(
+                    new OnRenameListener() {
 
-                @Override
-                public void onSuccess() {
-                    callback.success(conversation);
-                }
+                        @Override
+                        public void onSuccess() {
+                            callback.success(conversation);
+                        }
 
-                @Override
-                public void onFailure() {
-                    callback.error(R.string.nick_in_use, conversation);
-                }
-            });
+                        @Override
+                        public void onFailure() {
+                            callback.error(R.string.nick_in_use, conversation);
+                        }
+                    });
 
-            final var packet = mPresenceGenerator.selfPresence(account, Presence.Status.ONLINE, options.nonanonymous());
+            final var packet =
+                    mPresenceGenerator.selfPresence(
+                            account, Presence.Status.ONLINE, options.nonanonymous());
             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) {
-                Bookmark bookmark = conversation.getBookmark();
+            if (account.getStatus() == Account.State.ONLINE) {
                 if (bookmark != null) {
                     bookmark.setNick(nick);
-                    createBookmark(bookmark.getAccount(), bookmark);
+                    createBookmark(account, bookmark);
                 }
                 joinMuc(conversation);
             }
@@ -3382,6 +3407,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());
+        packet.setTo(joinJid);
+        sendPresencePacket(account, packet);
+    }
+
     public void leaveMuc(Conversation conversation) {
         leaveMuc(conversation, false);
     }