From e2ac1db225ac6abbb461dc62d58ee2c943fbae8c Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 10 Feb 2018 19:06:31 +0100 Subject: [PATCH] do not cross reference bookmarks and conversations --- .../siacs/conversations/entities/Account.java | 15 ++++++----- .../conversations/entities/Bookmark.java | 26 +++++++++---------- .../conversations/entities/Conversation.java | 17 ++---------- .../conversations/entities/MucOptions.java | 2 +- .../conversations/parser/PresenceParser.java | 4 +-- .../services/XmppConnectionService.java | 13 ++++++---- .../ui/ConferenceDetailsActivity.java | 5 ++-- .../ui/StartConversationActivity.java | 6 ++--- 8 files changed, 38 insertions(+), 50 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/entities/Account.java b/src/main/java/eu/siacs/conversations/entities/Account.java index e716ebda6..7da0433f2 100644 --- a/src/main/java/eu/siacs/conversations/entities/Account.java +++ b/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 bookmarks) { + public void setBookmarks(final CopyOnWriteArrayList 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) { diff --git a/src/main/java/eu/siacs/conversations/entities/Bookmark.java b/src/main/java/eu/siacs/conversations/entities/Bookmark.java index e1c28dcd7..fb4359f59 100644 --- a/src/main/java/eu/siacs/conversations/entities/Bookmark.java +++ b/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; 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; - } } diff --git a/src/main/java/eu/siacs/conversations/entities/Conversation.java b/src/main/java/eu/siacs/conversations/entities/Conversation.java index c7ea2f8a4..461f3b9aa 100644 --- a/src/main/java/eu/siacs/conversations/entities/Conversation.java +++ b/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) { diff --git a/src/main/java/eu/siacs/conversations/entities/MucOptions.java b/src/main/java/eu/siacs/conversations/entities/MucOptions.java index 3368ee830..35f026fd4 100644 --- a/src/main/java/eu/siacs/conversations/entities/MucOptions.java +++ b/src/main/java/eu/siacs/conversations/entities/MucOptions.java @@ -54,7 +54,7 @@ public class MucOptions { } public boolean isSelf(Jid counterpart) { - return counterpart.getResourcepart().equals(getActualNick()); + return counterpart.equals(self.getFullJid()); } public void resetChatState() { diff --git a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java b/src/main/java/eu/siacs/conversations/parser/PresenceParser.java index 2d1db79bf..a05b1aac2 100644 --- a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java +++ b/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); diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 85cce6cfa..638855e80 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/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 fingerprints) { diff --git a/src/main/java/eu/siacs/conversations/ui/ConferenceDetailsActivity.java b/src/main/java/eu/siacs/conversations/ui/ConferenceDetailsActivity.java index f4cf3182d..139ab410d 100644 --- a/src/main/java/eu/siacs/conversations/ui/ConferenceDetailsActivity.java +++ b/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(); } diff --git a/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java b/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java index 3bc9b7751..fd125609f 100644 --- a/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java +++ b/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);