do not cross reference bookmarks and conversations

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Account.java               | 15 
src/main/java/eu/siacs/conversations/entities/Bookmark.java              | 26 
src/main/java/eu/siacs/conversations/entities/Conversation.java          | 17 
src/main/java/eu/siacs/conversations/entities/MucOptions.java            |  2 
src/main/java/eu/siacs/conversations/parser/PresenceParser.java          |  4 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java | 13 
src/main/java/eu/siacs/conversations/ui/ConferenceDetailsActivity.java   |  5 
src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java   |  6 
8 files changed, 38 insertions(+), 50 deletions(-)

Detailed changes

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

@@ -610,18 +610,21 @@ public class Account extends AbstractEntity {
 		return this.bookmarks;
 	}
 
-	public void setBookmarks(final List<Bookmark> bookmarks) {
+	public void setBookmarks(final CopyOnWriteArrayList<Bookmark> bookmarks) {
 		this.bookmarks = bookmarks;
 	}
 
 	public boolean hasBookmarkFor(final Jid conferenceJid) {
-		for (final Bookmark bookmark : this.bookmarks) {
-			final Jid jid = bookmark.getJid();
-			if (jid != null && jid.equals(conferenceJid.toBareJid())) {
-				return true;
+		return getBookmark(conferenceJid) != null;
+	}
+
+	public Bookmark getBookmark(final Jid jid) {
+		for(final Bookmark bookmark : this.bookmarks) {
+			if (bookmark.getJid() != null && jid.toBareJid().equals(bookmark.getJid().toBareJid())) {
+				return bookmark;
 			}
 		}
-		return false;
+		return null;
 	}
 
 	public boolean setAvatar(final String filename) {

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

@@ -2,6 +2,7 @@ package eu.siacs.conversations.entities;
 
 import android.content.Context;
 
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
@@ -13,7 +14,7 @@ import eu.siacs.conversations.xmpp.jid.Jid;
 public class Bookmark extends Element implements ListItem {
 
 	private Account account;
-	private Conversation mJoinedConversation;
+	private WeakReference<Conversation> conversation;
 
 	public Bookmark(final Account account, final Jid jid) {
 		super("conference");
@@ -49,8 +50,9 @@ public class Bookmark extends Element implements ListItem {
 
 	@Override
 	public String getDisplayName() {
-		if (this.mJoinedConversation != null) {
-			return this.mJoinedConversation.getName();
+		final Conversation c = getConversation();
+		if (c != null) {
+			return c.getName();
 		} else if (getBookmarkName() != null
 				&& !getBookmarkName().trim().isEmpty()) {
 			return getBookmarkName().trim();
@@ -141,12 +143,15 @@ public class Bookmark extends Element implements ListItem {
 		return this.account;
 	}
 
-	public Conversation getConversation() {
-		return this.mJoinedConversation;
+	public synchronized Conversation getConversation() {
+		return this.conversation != null ? this.conversation.get() : null;
 	}
 
-	public void setConversation(Conversation conversation) {
-		this.mJoinedConversation = conversation;
+	public synchronized void setConversation(Conversation conversation) {
+		if (this.conversation != null) {
+			this.conversation.clear();
+		}
+		this.conversation = new WeakReference<>(conversation);
 	}
 
 	public String getBookmarkName() {
@@ -162,11 +167,4 @@ public class Bookmark extends Element implements ListItem {
 			return false;
 		}
 	}
-
-	public void unregisterConversation() {
-		if (this.mJoinedConversation != null) {
-			this.mJoinedConversation.deregisterWithBookmark();
-		}
-		this.mJoinedConversation = null;
-	}
 }

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

@@ -86,8 +86,6 @@ public class Conversation extends AbstractEntity implements Blockable, Comparabl
 
 	private byte[] symmetricKey;
 
-	private Bookmark bookmark;
-
 	private boolean messagesLeftOnServer = true;
 	private ChatState mOutgoingChatState = Config.DEFAULT_CHATSTATE;
 	private ChatState mIncomingChatState = Config.DEFAULT_CHATSTATE;
@@ -494,6 +492,7 @@ public class Conversation extends AbstractEntity implements Blockable, Comparabl
 	public String getName() {
 		if (getMode() == MODE_MULTI) {
 			final String subject = getMucOptions().getSubject();
+			Bookmark bookmark = getBookmark();
 			final String bookmarkName = bookmark != null ? bookmark.getBookmarkName() : null;
 			if (subject != null && !subject.trim().isEmpty()) {
 				return subject;
@@ -790,20 +789,8 @@ public class Conversation extends AbstractEntity implements Blockable, Comparabl
 		return this.symmetricKey;
 	}
 
-	public void setBookmark(Bookmark bookmark) {
-		this.bookmark = bookmark;
-		this.bookmark.setConversation(this);
-	}
-
-	public void deregisterWithBookmark() {
-		if (this.bookmark != null) {
-			this.bookmark.setConversation(null);
-		}
-		this.bookmark = null;
-	}
-
 	public Bookmark getBookmark() {
-		return this.bookmark;
+		return this.account.getBookmark(this.contactJid);
 	}
 
 	public Message findDuplicateMessage(Message message) {

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

@@ -69,15 +69,13 @@ public class PresenceParser extends AbstractParser implements
 					if (item != null && !from.isBareJid()) {
 						mucOptions.setError(MucOptions.Error.NONE);
 						MucOptions.User user = parseItem(conversation, item, from);
-						if (codes.contains(MucOptions.STATUS_CODE_SELF_PRESENCE)
-								|| ((codes.isEmpty() || codes.contains(MucOptions.STATUS_CODE_ROOM_CREATED)) && jid.equals(item.getAttributeAsJid("jid")))) {
+						if (codes.contains(MucOptions.STATUS_CODE_SELF_PRESENCE) || (codes.contains(MucOptions.STATUS_CODE_ROOM_CREATED) && jid.equals(item.getAttributeAsJid("jid")))) {
 							if (mucOptions.setOnline()) {
 								mXmppConnectionService.getAvatarService().clear(mucOptions);
 							}
 							mucOptions.setSelf(user);
 
 							mXmppConnectionService.persistSelfNick(user);
-
 							invokeRenameListener(mucOptions, true);
 						}
 						boolean isNew = mucOptions.updateUser(user);

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

@@ -1424,15 +1424,15 @@ public class XmppConnectionService extends Service {
 								}
 								Conversation conversation = find(bookmark);
 								if (conversation != null) {
-									conversation.setBookmark(bookmark);
+									bookmark.setConversation(conversation);
 								} else if (bookmark.autojoin() && bookmark.getJid() != null && autojoin) {
 									conversation = findOrCreateConversation(account, bookmark.getJid(), true, true, false);
-									conversation.setBookmark(bookmark);
+									bookmark.setConversation(conversation);
 								}
 							}
 						}
 					}
-					account.setBookmarks(new ArrayList<>(bookmarks.values()));
+					account.setBookmarks(new CopyOnWriteArrayList<>(bookmarks.values()));
 				} else {
 					Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": could not fetch bookmarks");
 				}
@@ -2478,7 +2478,10 @@ public class XmppConnectionService extends Service {
 		if (account.getStatus() == Account.State.ONLINE || now) {
 			sendPresencePacket(conversation.getAccount(), mPresenceGenerator.leave(conversation.getMucOptions()));
 			conversation.getMucOptions().setOffline();
-			conversation.deregisterWithBookmark();
+			Bookmark bookmark = conversation.getBookmark();
+			if (bookmark != null) {
+				bookmark.setConversation(null);
+			}
 			Log.d(Config.LOGTAG, conversation.getAccount().getJid().toBareJid() + ": leaving muc " + conversation.getJid());
 		} else {
 			account.pendingConferenceLeaves.add(conversation);
@@ -3948,7 +3951,7 @@ public class XmppConnectionService extends Service {
 		bookmark.setAutojoin(getPreferences().getBoolean("autojoin", getResources().getBoolean(R.bool.autojoin)));
 		account.getBookmarks().add(bookmark);
 		pushBookmarks(account);
-		conversation.setBookmark(bookmark);
+		bookmark.setConversation(conversation);
 	}
 
 	public boolean verifyFingerprints(Contact contact, List<XmppUri.Fingerprint> fingerprints) {

src/main/java/eu/siacs/conversations/ui/ConferenceDetailsActivity.java 🔗

@@ -347,8 +347,7 @@ public class ConferenceDetailsActivity extends XmppActivity implements OnConvers
 		if (mConversation == null) {
 			return true;
 		}
-		Account account = mConversation.getAccount();
-		if (account.hasBookmarkFor(mConversation.getJid().toBareJid())) {
+		if (mConversation.getBookmark() != null) {
 			menuItemSaveBookmark.setVisible(false);
 			menuItemDeleteBookmark.setVisible(true);
 		} else {
@@ -515,8 +514,8 @@ public class ConferenceDetailsActivity extends XmppActivity implements OnConvers
 	protected void deleteBookmark() {
 		Account account = mConversation.getAccount();
 		Bookmark bookmark = mConversation.getBookmark();
-		mConversation.deregisterWithBookmark();
 		account.getBookmarks().remove(bookmark);
+		bookmark.setConversation(null);
 		xmppConnectionService.pushBookmarks(account);
 		updateView();
 	}

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

@@ -342,7 +342,7 @@ public class StartConversationActivity extends XmppActivity implements OnRosterU
             return;
         }
         Conversation conversation = xmppConnectionService.findOrCreateConversation(bookmark.getAccount(), jid, true, true, true);
-        conversation.setBookmark(bookmark);
+        bookmark.setConversation(conversation);
         if (!bookmark.autojoin() && getPreferences().getBoolean("autojoin", getResources().getBoolean(R.bool.autojoin))) {
             bookmark.setAutojoin(true);
             xmppConnectionService.pushBookmarks(bookmark.getAccount());
@@ -393,7 +393,7 @@ public class StartConversationActivity extends XmppActivity implements OnRosterU
 
             @Override
             public void onClick(DialogInterface dialog, int which) {
-                bookmark.unregisterConversation();
+                bookmark.setConversation(null);
                 Account account = bookmark.getAccount();
                 account.getBookmarks().remove(bookmark);
                 xmppConnectionService.pushBookmarks(account);
@@ -498,7 +498,7 @@ public class StartConversationActivity extends XmppActivity implements OnRosterU
                                 xmppConnectionService.pushBookmarks(account);
                                 final Conversation conversation = xmppConnectionService
                                         .findOrCreateConversation(account, conferenceJid, true, true, true);
-                                conversation.setBookmark(bookmark);
+                                bookmark.setConversation(conversation);
                                 dialog.dismiss();
                                 mCurrentDialog = null;
                                 switchToConversation(conversation);