Bookmarks2: support retraction

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Account.java               | 55 
src/main/java/eu/siacs/conversations/entities/Bookmark.java              | 18 
src/main/java/eu/siacs/conversations/generator/IqGenerator.java          |  1 
src/main/java/eu/siacs/conversations/parser/MessageParser.java           | 24 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java | 16 
src/main/java/eu/siacs/conversations/ui/ChannelDiscoveryActivity.java    |  1 
src/main/java/eu/siacs/conversations/ui/ConferenceDetailsActivity.java   |  5 
src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java   |  4 
8 files changed, 78 insertions(+), 46 deletions(-)

Detailed changes

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

@@ -11,8 +11,10 @@ import org.json.JSONObject;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CopyOnWriteArraySet;
@@ -84,7 +86,7 @@ public class Account extends AbstractEntity implements AvatarService.Avatarable
     private PgpDecryptionService pgpDecryptionService = null;
     private XmppConnection xmppConnection = null;
     private long mEndGracePeriod = 0L;
-    private List<Bookmark> bookmarks = new CopyOnWriteArrayList<>();
+    private final Map<Jid, Bookmark> bookmarks = new HashMap<>();
     private Presence.Status presenceStatus = Presence.Status.ONLINE;
     private String presenceStatusMessage = null;
 
@@ -469,36 +471,51 @@ public class Account extends AbstractEntity implements AvatarService.Avatarable
         return this.roster;
     }
 
-    public List<Bookmark> getBookmarks() {
-        return this.bookmarks;
+    public Collection<Bookmark> getBookmarks() {
+        return this.bookmarks.values();
     }
 
-    public void setBookmarks(final CopyOnWriteArrayList<Bookmark> bookmarks) {
-        this.bookmarks = bookmarks;
+    public void setBookmarks(Map<Jid, Bookmark> bookmarks) {
+        synchronized (this.bookmarks) {
+            this.bookmarks.clear();
+            this.bookmarks.putAll(bookmarks);
+        }
+    }
+
+    public void putBookmark(Bookmark bookmark) {
+        synchronized (this.bookmarks) {
+            this.bookmarks.put(bookmark.getJid(), bookmark);
+        }
+    }
+
+    public void removeBookmark(Bookmark bookmark) {
+        synchronized (this.bookmarks) {
+            this.bookmarks.remove(bookmark.getJid());
+        }
+    }
+
+    public void removeBookmark(Jid jid) {
+        synchronized (this.bookmarks) {
+            this.bookmarks.remove(jid);
+        }
     }
 
     public Set<Jid> getBookmarkedJids() {
-        final Set<Jid> jids = new HashSet<>();
-        for(final Bookmark bookmark : this.bookmarks) {
-            final Jid jid = bookmark.getJid();
-            if (jid != null) {
-                jids.add(jid.asBareJid());
-            }
+        synchronized (this.bookmarks) {
+            return new HashSet<>(this.bookmarks.keySet());
         }
-        return jids;
     }
 
-    public boolean hasBookmarkFor(final Jid conferenceJid) {
-        return getBookmark(conferenceJid) != null;
+    public boolean hasBookmarkFor(final Jid jid) {
+        synchronized (this.bookmarks) {
+            return this.bookmarks.containsKey(jid.asBareJid());
+        }
     }
 
     Bookmark getBookmark(final Jid jid) {
-        for (final Bookmark bookmark : this.bookmarks) {
-            if (bookmark.getJid() != null && jid.asBareJid().equals(bookmark.getJid().asBareJid())) {
-                return bookmark;
-            }
+        synchronized (this.bookmarks) {
+            return this.bookmarks.get(jid.asBareJid());
         }
-        return null;
     }
 
     public boolean setAvatar(final String filename) {

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

@@ -41,43 +41,43 @@ public class Bookmark extends Element implements ListItem {
 		this.account = account;
 	}
 
-	public static Collection<Bookmark> parseFromStorage(Element storage, Account account) {
+	public static Map<Jid, Bookmark> parseFromStorage(Element storage, Account account) {
 		if (storage == null) {
-			return Collections.emptyList();
+			return Collections.emptyMap();
 		}
 		final HashMap<Jid, Bookmark> bookmarks = new HashMap<>();
 		for (final Element item : storage.getChildren()) {
 			if (item.getName().equals("conference")) {
 				final Bookmark bookmark = Bookmark.parse(item, account);
 				if (bookmark != null) {
-					final Bookmark old = bookmarks.put(bookmark.getJid(), bookmark);
+					final Bookmark old = bookmarks.put(bookmark.jid, bookmark);
 					if (old != null && old.getBookmarkName() != null && bookmark.getBookmarkName() == null) {
 						bookmark.setBookmarkName(old.getBookmarkName());
 					}
 				}
 			}
 		}
-		return bookmarks.values();
+		return bookmarks;
 	}
 
-	public static Collection<Bookmark> parseFromPubsub(Element pubsub, Account account) {
+	public static Map<Jid, Bookmark> parseFromPubsub(Element pubsub, Account account) {
 		if (pubsub == null) {
-			return Collections.emptyList();
+			return Collections.emptyMap();
 		}
 		final Element items = pubsub.findChild("items");
 		if (items != null && Namespace.BOOKMARK.equals(items.getAttribute("node"))) {
-			final List<Bookmark> bookmarks = new ArrayList<>();
+			final Map<Jid, Bookmark> bookmarks = new HashMap<>();
 			for(Element item : items.getChildren()) {
 				if (item.getName().equals("item")) {
 					final Bookmark bookmark = Bookmark.parseFromItem(item, account);
 					if (bookmark != null) {
-						bookmarks.add(bookmark);
+						bookmarks.put(bookmark.jid, bookmark);
 					}
 				}
 			}
 			return bookmarks;
 		}
-		return Collections.emptyList();
+		return Collections.emptyMap();
 	}
 
 	public static Bookmark parse(Element element, Account account) {

src/main/java/eu/siacs/conversations/generator/IqGenerator.java 🔗

@@ -147,6 +147,7 @@ public class IqGenerator extends AbstractGenerator {
         final Element pubsub = packet.addChild("pubsub", Namespace.PUBSUB);
         final Element retract = pubsub.addChild("retract");
         retract.setAttribute("node", node);
+        retract.setAttribute("notify","true");
         retract.addChild("item").setAttribute("id", id);
         return packet;
     }

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

@@ -227,11 +227,29 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
             if (account.getXmppConnection().getFeatures().bookmarksConversion()) {
                 final Element i = items.findChild("item");
                 final Element storage = i == null ? null : i.findChild("storage", Namespace.BOOKMARKS);
-                Collection<Bookmark> bookmarks = Bookmark.parseFromStorage(storage, account);
+                Map<Jid, Bookmark> bookmarks = Bookmark.parseFromStorage(storage, account);
                 mXmppConnectionService.processBookmarksInitial(account, bookmarks, true);
-                Log.d(Config.LOGTAG,account.getJid().asBareJid()+": processing bookmark PEP event");
+                Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": processing bookmark PEP event");
             } else {
-                Log.d(Config.LOGTAG,account.getJid().asBareJid()+": ignoring bookmark PEP event because bookmark conversion was not detected");
+                Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": ignoring bookmark PEP event because bookmark conversion was not detected");
+            }
+        } else if (Namespace.BOOKMARK.equals(node) && account.getJid().asBareJid().equals(from)) {
+            final Element item = items.findChild("item");
+            final Element retract = items.findChild("retract");
+            if (item != null) {
+                final Bookmark bookmark = Bookmark.parseFromItem(item, account);
+                if (bookmark != null) {
+                    //TODO find conversation
+                    account.putBookmark(bookmark);
+                    //TODO handle autojoin
+                }
+            }
+            if (retract != null) {
+                final Jid id = InvalidJid.getNullForInvalid(retract.getAttributeAsJid("id"));
+                if (id != null) {
+                    account.removeBookmark(id);
+                    Log.d(Config.LOGTAG,account.getJid().asBareJid()+": deleted bookmark for "+id);
+                }
             }
         } else {
             Log.d(Config.LOGTAG,account.getJid().asBareJid()+" received pubsub notification for node="+node);

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

@@ -1564,7 +1564,7 @@ public class XmppConnectionService extends Service {
             if (response.getType() == IqPacket.TYPE.RESULT) {
                 final Element query1 = response.query();
                 final Element storage = query1.findChild("storage", "storage:bookmarks");
-                Collection<Bookmark> bookmarks = Bookmark.parseFromStorage(storage, account);
+                Map<Jid, Bookmark> bookmarks = Bookmark.parseFromStorage(storage, account);
                 processBookmarksInitial(a, bookmarks, false);
             } else {
                 Log.d(Config.LOGTAG, a.getJid().asBareJid() + ": could not fetch bookmarks");
@@ -1580,17 +1580,17 @@ public class XmppConnectionService extends Service {
             public void onIqPacketReceived(final Account account, final IqPacket response) {
                 if (response.getType() == IqPacket.TYPE.RESULT) {
                     final Element pubsub = response.findChild("pubsub", Namespace.PUBSUB);
-                    final Collection<Bookmark> bookmarks = Bookmark.parseFromPubsub(pubsub, account);
+                    final Map<Jid, Bookmark> bookmarks = Bookmark.parseFromPubsub(pubsub, account);
                     processBookmarksInitial(account, bookmarks, true);
                 }
             }
         });
     }
 
-    public void processBookmarksInitial(Account account, Collection<Bookmark> bookmarks, final boolean pep) {
+    public void processBookmarksInitial(Account account, Map<Jid,Bookmark> bookmarks, final boolean pep) {
         final Set<Jid> previousBookmarks = account.getBookmarkedJids();
         final boolean synchronizeWithBookmarks = synchronizeWithBookmarks();
-        for (Bookmark bookmark : bookmarks) {
+        for (Bookmark bookmark : bookmarks.values()) {
             previousBookmarks.remove(bookmark.getJid().asBareJid());
             Conversation conversation = find(bookmark);
             if (conversation != null) {
@@ -1617,10 +1617,11 @@ public class XmppConnectionService extends Service {
                 }
             }
         }
-        account.setBookmarks(new CopyOnWriteArrayList<>(bookmarks));
+        account.setBookmarks(bookmarks);
     }
 
     public void createBookmark(final Account account, final Bookmark bookmark) {
+        account.putBookmark(bookmark);
         final XmppConnection connection = account.getXmppConnection();
         if (connection.getFeatures().bookmarks2()) {
             final Element item = mIqGenerator.publishBookmarkItem(bookmark);
@@ -1633,6 +1634,7 @@ public class XmppConnectionService extends Service {
     }
 
     public void deleteBookmark(final Account account, final Bookmark bookmark) {
+        account.removeBookmark(bookmark);
         final XmppConnection connection = account.getXmppConnection();
         if (connection.getFeatures().bookmarksConversion()) {
             IqPacket request = mIqGenerator.deleteItem(Namespace.BOOKMARK, bookmark.getJid().asBareJid().toEscapedString());
@@ -2068,12 +2070,11 @@ public class XmppConnectionService extends Service {
 			getMessageArchiveService().kill(conversation);
 			if (conversation.getMode() == Conversation.MODE_MULTI) {
 				if (conversation.getAccount().getStatus() == Account.State.ONLINE) {
-					Bookmark bookmark = conversation.getBookmark();
+					final Bookmark bookmark = conversation.getBookmark();
 					if (maySynchronizeWithBookmarks && bookmark != null && synchronizeWithBookmarks()) {
 						if (conversation.getMucOptions().getError() == MucOptions.Error.DESTROYED) {
 							Account account = bookmark.getAccount();
 							bookmark.setConversation(null);
-							account.getBookmarks().remove(bookmark);
 							deleteBookmark(account, bookmark);
 						} else if (bookmark.autojoin()) {
 							bookmark.setAutojoin(false);
@@ -4473,7 +4474,6 @@ public class XmppConnectionService extends Service {
 			bookmark.setBookmarkName(name);
 		}
 		bookmark.setAutojoin(getPreferences().getBoolean("autojoin", getResources().getBoolean(R.bool.autojoin)));
-		account.getBookmarks().add(bookmark);
 		createBookmark(account, bookmark);
 		bookmark.setConversation(conversation);
 	}

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

@@ -232,7 +232,6 @@ public class ChannelDiscoveryActivity extends XmppActivity implements MenuItem.O
         } else {
             bookmark = new Bookmark(account, conversation.getJid().asBareJid());
             bookmark.setAutojoin(syncAutojoin);
-            account.getBookmarks().add(bookmark);
             xmppConnectionService.createBookmark(account, bookmark);
         }
         switchToConversation(conversation);

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

@@ -386,9 +386,8 @@ public class ConferenceDetailsActivity extends XmppActivity implements OnConvers
     }
 
     protected void deleteBookmark() {
-        Account account = mConversation.getAccount();
-        Bookmark bookmark = mConversation.getBookmark();
-        account.getBookmarks().remove(bookmark);
+        final Account account = mConversation.getAccount();
+        final Bookmark bookmark = mConversation.getBookmark();
         bookmark.setConversation(null);
         xmppConnectionService.deleteBookmark(account, bookmark);
         updateView();

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

@@ -478,8 +478,7 @@ public class StartConversationActivity extends XmppActivity implements XmppConne
 		builder.setMessage(JidDialog.style(this, R.string.remove_bookmark_text, bookmark.getJid().toEscapedString()));
 		builder.setPositiveButton(R.string.delete, (dialog, which) -> {
 			bookmark.setConversation(null);
-			Account account = bookmark.getAccount();
-			account.getBookmarks().remove(bookmark);
+			final Account account = bookmark.getAccount();
 			xmppConnectionService.deleteBookmark(account, bookmark);
 			filter(mSearchEditText.getText().toString());
 		});
@@ -1041,7 +1040,6 @@ public class StartConversationActivity extends XmppActivity implements XmppConne
 				if (nick != null && !nick.isEmpty() && !nick.equals(MucOptions.defaultNick(account))) {
 					bookmark.setNick(nick);
 				}
-				account.getBookmarks().add(bookmark);
 				xmppConnectionService.createBookmark(account, bookmark);
 				final Conversation conversation = xmppConnectionService
 						.findOrCreateConversation(account, conferenceJid, true, true, true);